linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes
@ 2020-08-22 15:08 Guenter Roeck
  2020-08-22 15:08 ` [PATCH v5 1/7] iio: cros_ec: Accept -EOPNOTSUPP as 'not supported' error code Guenter Roeck
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Guenter Roeck @ 2020-08-22 15:08 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Jonathan Cameron, Benson Leung, Dmitry Torokhov, Thierry Reding,
	Uwe Kleine-König, Lee Jones, Gwendal Grignou, Yu-Hsuan Hsu,
	Prashant Malani, linux-iio, linux-input, linux-pwm, linux-kernel,
	Guenter Roeck

The EC reports a variety of error codes. Most of those, with the exception
of EC_RES_INVALID_VERSION, are converted to -EPROTO. As result, the actual
error code gets lost. In cros_ec_cmd_xfer_status(), convert all EC errors
to Linux error codes to report a more meaningful error to the caller to aid
debugging.

To prepare for this change, handle error codes other than -EPROTO for all
callers of cros_ec_cmd_xfer_status(). Specifically, no longer assume that
-EPROTO reflects an error from the EC and all other error codes reflect a
transfer error.

v2: Add patches 1/4 to 3/4 to handle callers of cros_ec_cmd_xfer_status()
v3: Add patches 4/6 and 5/6 to handle additional callers of
	cros_ec_cmd_xfer_status()
    Use -ENOPROTOOPT for EC_RES_INVALID_VERSION
    Implement function to convert error codes
v4: Add coments describing the functionality of cros_ec_num_pwms().
    Add patch 7/7 to clean up cros_ec_num_pwms() after the new error code
    support has been implemented.
    Rebased series to v5.8.
v5: If there is no error, cros_ec_cmd_xfer_status() needs to return the
    number of received bytes, not 0. While fixing that (in patch 6/7),
    rearranged the function to reduce its indentation.
    Rebased series to v5.9-rc1

----------------------------------------------------------------
Guenter Roeck (7):
      iio: cros_ec: Accept -EOPNOTSUPP as 'not supported' error code
      cros_ec_lightbar: Accept more error codes from cros_ec_cmd_xfer_status
      platform/chrome: cros_ec_sysfs: Report range of error codes from EC
      pwm: cros-ec: Accept more error codes from cros_ec_cmd_xfer_status
      platform/input: cros_ec: Replace -ENOTSUPP with -ENOPROTOOPT
      platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes
      pwm: cros-ec: Simplify EC error handling

 .../iio/common/cros_ec_sensors/cros_ec_sensors.c   |  2 +-
 drivers/input/keyboard/cros_ec_keyb.c              |  2 +-
 drivers/platform/chrome/cros_ec_lightbar.c         | 10 ++--
 drivers/platform/chrome/cros_ec_proto.c            | 57 +++++++++++++++++-----
 drivers/platform/chrome/cros_ec_sysfs.c            | 24 ++++-----
 drivers/pwm/pwm-cros-ec.c                          | 37 +++++++-------
 6 files changed, 78 insertions(+), 54 deletions(-)

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

* [PATCH v5 1/7] iio: cros_ec: Accept -EOPNOTSUPP as 'not supported' error code
  2020-08-22 15:08 [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Guenter Roeck
@ 2020-08-22 15:08 ` Guenter Roeck
  2020-08-22 15:08 ` [PATCH v5 2/7] cros_ec_lightbar: Accept more error codes from cros_ec_cmd_xfer_status Guenter Roeck
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2020-08-22 15:08 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Jonathan Cameron, Benson Leung, Dmitry Torokhov, Thierry Reding,
	Uwe Kleine-König, Lee Jones, Gwendal Grignou, Yu-Hsuan Hsu,
	Prashant Malani, linux-iio, linux-input, linux-pwm, linux-kernel,
	Guenter Roeck, Brian Norris

A follow-up patch will extend the number of errors reported by
cros_ec_cmd_xfer_status(). Specifically, the function will return
-EOPNOTSUPP if a command is not supported by the EC. Prepare for it.

Cc: Gwendal Grignou <gwendal@chromium.org>
Cc: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Cc: Prashant Malani <pmalani@chromium.org>
Cc: Brian Norris <briannorris@chromium.org>
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: No change
v4: No change
v3: No change
v2: No change

 drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
index 130ab8ce0269..d71e9064c789 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
@@ -73,7 +73,7 @@ static int cros_ec_sensors_read(struct iio_dev *indio_dev,
 		st->core.param.sensor_offset.flags = 0;
 
 		ret = cros_ec_motion_send_host_cmd(&st->core, 0);
-		if (ret == -EPROTO) {
+		if (ret == -EPROTO || ret == -EOPNOTSUPP) {
 			/* Reading calibscale is not supported on older EC. */
 			*val = 1;
 			*val2 = 0;
-- 
2.17.1


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

* [PATCH v5 2/7] cros_ec_lightbar: Accept more error codes from cros_ec_cmd_xfer_status
  2020-08-22 15:08 [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Guenter Roeck
  2020-08-22 15:08 ` [PATCH v5 1/7] iio: cros_ec: Accept -EOPNOTSUPP as 'not supported' error code Guenter Roeck
@ 2020-08-22 15:08 ` Guenter Roeck
  2020-08-22 15:08 ` [PATCH v5 3/7] platform/chrome: cros_ec_sysfs: Report range of error codes from EC Guenter Roeck
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2020-08-22 15:08 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Jonathan Cameron, Benson Leung, Dmitry Torokhov, Thierry Reding,
	Uwe Kleine-König, Lee Jones, Gwendal Grignou, Yu-Hsuan Hsu,
	Prashant Malani, linux-iio, linux-input, linux-pwm, linux-kernel,
	Guenter Roeck, Brian Norris

Since commit c5cd2b47b203 ("platform/chrome: cros_ec_proto: Report command
not supported") we can no longer assume that cros_ec_cmd_xfer_status()
reports -EPROTO for all errors returned by the EC itself. A follow-up
patch will change cros_ec_cmd_xfer_status() to report additional errors
reported by the EC as distinguished Linux error codes.

Handle this change by no longer assuming that -EPROTO is used to report
all errors returned by the EC itself. Since errors reported by the EC are
already reported in text form through sysfs attributes, extend this form
of error reporting to all errors reported by cros_ec_cmd_xfer_status().

Cc: Gwendal Grignou <gwendal@chromium.org>
Cc: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Cc: Prashant Malani <pmalani@chromium.org>
Cc: Brian Norris <briannorris@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: No change
v4: No change
v3: No change
v2: Added patch

 drivers/platform/chrome/cros_ec_lightbar.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index b59180bff5a3..8445cda57927 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -117,7 +117,7 @@ static int get_lightbar_version(struct cros_ec_dev *ec,
 	param = (struct ec_params_lightbar *)msg->data;
 	param->cmd = LIGHTBAR_CMD_VERSION;
 	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
-	if (ret < 0) {
+	if (ret < 0 && ret != -EINVAL) {
 		ret = 0;
 		goto exit;
 	}
@@ -298,11 +298,9 @@ static ssize_t sequence_show(struct device *dev,
 		goto exit;
 
 	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
-	if (ret == -EPROTO) {
-		ret = scnprintf(buf, PAGE_SIZE,
-				"ERROR: EC returned %d\n", msg->result);
-		goto exit;
-	} else if (ret < 0) {
+	if (ret < 0) {
+		ret = scnprintf(buf, PAGE_SIZE, "XFER / EC ERROR %d / %d\n",
+				ret, msg->result);
 		goto exit;
 	}
 
-- 
2.17.1


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

* [PATCH v5 3/7] platform/chrome: cros_ec_sysfs: Report range of error codes from EC
  2020-08-22 15:08 [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Guenter Roeck
  2020-08-22 15:08 ` [PATCH v5 1/7] iio: cros_ec: Accept -EOPNOTSUPP as 'not supported' error code Guenter Roeck
  2020-08-22 15:08 ` [PATCH v5 2/7] cros_ec_lightbar: Accept more error codes from cros_ec_cmd_xfer_status Guenter Roeck
@ 2020-08-22 15:08 ` Guenter Roeck
  2020-08-22 15:08 ` [PATCH v5 4/7] pwm: cros-ec: Accept more error codes from cros_ec_cmd_xfer_status Guenter Roeck
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2020-08-22 15:08 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Jonathan Cameron, Benson Leung, Dmitry Torokhov, Thierry Reding,
	Uwe Kleine-König, Lee Jones, Gwendal Grignou, Yu-Hsuan Hsu,
	Prashant Malani, linux-iio, linux-input, linux-pwm, linux-kernel,
	Guenter Roeck, Brian Norris

Since commit c5cd2b47b203 ("platform/chrome: cros_ec_proto: Report command
not supported") we can no longer assume that cros_ec_cmd_xfer_status()
reports -EPROTO for all errors returned by the EC itself. A follow-up
patch will change cros_ec_cmd_xfer_status() to report additional errors
reported by the EC as distinguished Linux error codes.

Prepare for this change by always reporting both the Linux error code
and the EC error code in sysfs attributes.

Cc: Gwendal Grignou <gwendal@chromium.org>
Cc: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Cc: Prashant Malani <pmalani@chromium.org>
Cc: Brian Norris <briannorris@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: No change
v4: No change
v3: No change
v2: Added patch

 drivers/platform/chrome/cros_ec_sysfs.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index d45ea5d5bfa4..9c1e0998a721 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -150,12 +150,10 @@ static ssize_t version_show(struct device *dev,
 	msg->command = EC_CMD_GET_BUILD_INFO + ec->cmd_offset;
 	msg->insize = EC_HOST_PARAM_SIZE;
 	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
-	if (ret == -EPROTO) {
-		count += scnprintf(buf + count, PAGE_SIZE - count,
-				   "Build info:    EC error %d\n", msg->result);
-	} else if (ret < 0) {
+	if (ret < 0) {
 		count += scnprintf(buf + count, PAGE_SIZE - count,
-				   "Build info:    XFER ERROR %d\n", ret);
+				   "Build info:    XFER / EC ERROR %d / %d\n",
+				   ret, msg->result);
 	} else {
 		msg->data[EC_HOST_PARAM_SIZE - 1] = '\0';
 		count += scnprintf(buf + count, PAGE_SIZE - count,
@@ -166,12 +164,10 @@ static ssize_t version_show(struct device *dev,
 	msg->command = EC_CMD_GET_CHIP_INFO + ec->cmd_offset;
 	msg->insize = sizeof(*r_chip);
 	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
-	if (ret == -EPROTO) {
-		count += scnprintf(buf + count, PAGE_SIZE - count,
-				   "Chip info:     EC error %d\n", msg->result);
-	} else if (ret < 0) {
+	if (ret < 0) {
 		count += scnprintf(buf + count, PAGE_SIZE - count,
-				   "Chip info:     XFER ERROR %d\n", ret);
+				   "Chip info:     XFER / EC ERROR %d / %d\n",
+				   ret, msg->result);
 	} else {
 		r_chip = (struct ec_response_get_chip_info *)msg->data;
 
@@ -190,12 +186,10 @@ static ssize_t version_show(struct device *dev,
 	msg->command = EC_CMD_GET_BOARD_VERSION + ec->cmd_offset;
 	msg->insize = sizeof(*r_board);
 	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
-	if (ret == -EPROTO) {
-		count += scnprintf(buf + count, PAGE_SIZE - count,
-				   "Board version: EC error %d\n", msg->result);
-	} else if (ret < 0) {
+	if (ret < 0) {
 		count += scnprintf(buf + count, PAGE_SIZE - count,
-				   "Board version: XFER ERROR %d\n", ret);
+				   "Board version: XFER / EC ERROR %d / %d\n",
+				   ret, msg->result);
 	} else {
 		r_board = (struct ec_response_board_version *)msg->data;
 
-- 
2.17.1


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

* [PATCH v5 4/7] pwm: cros-ec: Accept more error codes from cros_ec_cmd_xfer_status
  2020-08-22 15:08 [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Guenter Roeck
                   ` (2 preceding siblings ...)
  2020-08-22 15:08 ` [PATCH v5 3/7] platform/chrome: cros_ec_sysfs: Report range of error codes from EC Guenter Roeck
@ 2020-08-22 15:08 ` Guenter Roeck
  2020-08-22 15:08 ` [PATCH v5 5/7] platform/input: cros_ec: Replace -ENOTSUPP with -ENOPROTOOPT Guenter Roeck
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2020-08-22 15:08 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Jonathan Cameron, Benson Leung, Dmitry Torokhov, Thierry Reding,
	Uwe Kleine-König, Lee Jones, Gwendal Grignou, Yu-Hsuan Hsu,
	Prashant Malani, linux-iio, linux-input, linux-pwm, linux-kernel,
	Guenter Roeck, Brian Norris

Since commit c5cd2b47b203 ("platform/chrome: cros_ec_proto: Report command
not supported") we can no longer assume that cros_ec_cmd_xfer_status()
reports -EPROTO for all errors returned by the EC itself. A follow-up
patch will change cros_ec_cmd_xfer_status() to report additional errors
reported by the EC as distinguished Linux error codes.

Handle this change by no longer assuming that only -EPROTO is used
to report all errors returned by the EC itself. Instead, support both
the old and the new error codes.

Add a comment describing cros_ec_num_pwms() to explain its functionality.

Cc: Gwendal Grignou <gwendal@chromium.org>
Cc: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Cc: Prashant Malani <pmalani@chromium.org>
Cc: Brian Norris <briannorris@chromium.org>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: Added Uwe's Acked-by: tag
v4: Added comments describing cros_ec_num_pwms() in more detail
    Added Thierry's Acked-by: tag
v3: Added patch

 drivers/pwm/pwm-cros-ec.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
index 09c08dee099e..94d3dff9b0e5 100644
--- a/drivers/pwm/pwm-cros-ec.c
+++ b/drivers/pwm/pwm-cros-ec.c
@@ -204,6 +204,11 @@ static const struct pwm_ops cros_ec_pwm_ops = {
 	.owner		= THIS_MODULE,
 };
 
+/*
+ * Determine the number of supported PWMs. The EC does not return the number
+ * of PWMs it supports directly, so we have to read the pwm duty cycle for
+ * subsequent channels until we get an error.
+ */
 static int cros_ec_num_pwms(struct cros_ec_device *ec)
 {
 	int i, ret;
@@ -213,20 +218,30 @@ static int cros_ec_num_pwms(struct cros_ec_device *ec)
 		u32 result = 0;
 
 		ret = __cros_ec_pwm_get_duty(ec, i, &result);
-		/* We want to parse EC protocol errors */
-		if (ret < 0 && !(ret == -EPROTO && result))
-			return ret;
-
 		/*
 		 * We look for SUCCESS, INVALID_COMMAND, or INVALID_PARAM
 		 * responses; everything else is treated as an error.
+		 * The EC error codes either map to -EOPNOTSUPP / -EINVAL,
+		 * or -EPROTO is returned and the EC error is in the result
+		 * field. Check for both.
 		 */
-		if (result == EC_RES_INVALID_COMMAND)
+		switch (ret) {
+		case -EOPNOTSUPP:	/* invalid command */
 			return -ENODEV;
-		else if (result == EC_RES_INVALID_PARAM)
+		case -EINVAL:		/* invalid parameter */
 			return i;
-		else if (result)
+		case -EPROTO:
+			/* Old or new error return code: Handle both */
+			if (result == EC_RES_INVALID_COMMAND)
+				return -ENODEV;
+			else if (result == EC_RES_INVALID_PARAM)
+				return i;
 			return -EPROTO;
+		default:
+			if (ret < 0)
+				return ret;
+			break;
+		}
 	}
 
 	return U8_MAX;
-- 
2.17.1


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

* [PATCH v5 5/7] platform/input: cros_ec: Replace -ENOTSUPP with -ENOPROTOOPT
  2020-08-22 15:08 [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Guenter Roeck
                   ` (3 preceding siblings ...)
  2020-08-22 15:08 ` [PATCH v5 4/7] pwm: cros-ec: Accept more error codes from cros_ec_cmd_xfer_status Guenter Roeck
@ 2020-08-22 15:08 ` Guenter Roeck
  2020-08-22 15:08 ` [PATCH v5 6/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Guenter Roeck
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2020-08-22 15:08 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Jonathan Cameron, Benson Leung, Dmitry Torokhov, Thierry Reding,
	Uwe Kleine-König, Lee Jones, Gwendal Grignou, Yu-Hsuan Hsu,
	Prashant Malani, linux-iio, linux-input, linux-pwm, linux-kernel,
	Guenter Roeck, Brian Norris

-ENOTSUPP is not a SUSV4 error code and should not be used. Use
-ENOPROTOOPT instead to report EC_RES_INVALID_VERSION responses
from the EC. This matches match the NFS response for unsupported
protocol versions.

Cc: Gwendal Grignou <gwendal@chromium.org>
Cc: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Cc: Prashant Malani <pmalani@chromium.org>
Cc: Brian Norris <briannorris@chromium.org>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: No change
v4: Added Dmitry's Acked-by: tag
v3: Added patch

 drivers/input/keyboard/cros_ec_keyb.c   | 2 +-
 drivers/platform/chrome/cros_ec_proto.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index fc1793ca2f17..15d17c717081 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -348,7 +348,7 @@ static int cros_ec_keyb_info(struct cros_ec_device *ec_dev,
 	params->event_type = event_type;
 
 	ret = cros_ec_cmd_xfer_status(ec_dev, msg);
-	if (ret == -ENOTSUPP) {
+	if (ret == -ENOPROTOOPT) {
 		/* With older ECs we just return 0 for everything */
 		memset(result, 0, result_size);
 		ret = 0;
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 8d52b3b4bd4e..f4727511fb41 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -579,7 +579,7 @@ static int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
  *
  * Return:
  * >=0 - The number of bytes transferred
- * -ENOTSUPP - Operation not supported
+ * -ENOPROTOOPT - Operation not supported
  * -EPROTO - Protocol error
  */
 int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
@@ -593,7 +593,7 @@ int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
 	} else if (msg->result == EC_RES_INVALID_VERSION) {
 		dev_dbg(ec_dev->dev, "Command invalid version (err:%d)\n",
 			msg->result);
-		return -ENOTSUPP;
+		return -ENOPROTOOPT;
 	} else if (msg->result != EC_RES_SUCCESS) {
 		dev_dbg(ec_dev->dev, "Command result (err: %d)\n", msg->result);
 		return -EPROTO;
-- 
2.17.1


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

* [PATCH v5 6/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes
  2020-08-22 15:08 [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Guenter Roeck
                   ` (4 preceding siblings ...)
  2020-08-22 15:08 ` [PATCH v5 5/7] platform/input: cros_ec: Replace -ENOTSUPP with -ENOPROTOOPT Guenter Roeck
@ 2020-08-22 15:08 ` Guenter Roeck
  2020-08-22 15:08 ` [PATCH v5 7/7] pwm: cros-ec: Simplify EC error handling Guenter Roeck
  2020-09-01  9:10 ` [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Enric Balletbo i Serra
  7 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2020-08-22 15:08 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Jonathan Cameron, Benson Leung, Dmitry Torokhov, Thierry Reding,
	Uwe Kleine-König, Lee Jones, Gwendal Grignou, Yu-Hsuan Hsu,
	Prashant Malani, linux-iio, linux-input, linux-pwm, linux-kernel,
	Guenter Roeck, Brian Norris

The EC reports a variety of error codes. Most of those, with the exception
of EC_RES_INVALID_VERSION, are converted to -EPROTO. As result, the actual
EC error code gets lost. Introduce cros_ec_map_error() to map EC error
codes to Linux error codes, and use it in cros_ec_cmd_xfer_status() to
report more meaningful errors to the caller. With this change, callers of
cros_ec_cmd_xfer_status() can implement a more distinguished action without
having to rely on the EC error code. At the same time, debugging is improved
in situations where the Linux error code is reported to userspace and/or in
the kernel log.

Cc: Gwendal Grignou <gwendal@chromium.org>
Cc: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Cc: Prashant Malani <pmalani@chromium.org>
Cc: Brian Norris <briannorris@chromium.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: If there is no error, cros_ec_cmd_xfer_status() needs to return the
    number of received bytes, not 0. While fixing that, rearranged the
    function to reduce its indentation.
    Dropped Reviewed-by:/Acked-by: tags to reflect change
v4: Added Brian's Reviewed-by: tag
v3: Use -ENOPROTOOPT for EC_RES_INVALID_VERSION
    Implement function to convert error codes
v2: No change

 drivers/platform/chrome/cros_ec_proto.c | 57 ++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index f4727511fb41..dda182132a6a 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -15,6 +15,43 @@
 
 #define EC_COMMAND_RETRIES	50
 
+static const int cros_ec_error_map[] = {
+	[EC_RES_INVALID_COMMAND] = -EOPNOTSUPP,
+	[EC_RES_ERROR] = -EIO,
+	[EC_RES_INVALID_PARAM] = -EINVAL,
+	[EC_RES_ACCESS_DENIED] = -EACCES,
+	[EC_RES_INVALID_RESPONSE] = -EPROTO,
+	[EC_RES_INVALID_VERSION] = -ENOPROTOOPT,
+	[EC_RES_INVALID_CHECKSUM] = -EBADMSG,
+	[EC_RES_IN_PROGRESS] = -EINPROGRESS,
+	[EC_RES_UNAVAILABLE] = -ENODATA,
+	[EC_RES_TIMEOUT] = -ETIMEDOUT,
+	[EC_RES_OVERFLOW] = -EOVERFLOW,
+	[EC_RES_INVALID_HEADER] = -EBADR,
+	[EC_RES_REQUEST_TRUNCATED] = -EBADR,
+	[EC_RES_RESPONSE_TOO_BIG] = -EFBIG,
+	[EC_RES_BUS_ERROR] = -EFAULT,
+	[EC_RES_BUSY] = -EBUSY,
+	[EC_RES_INVALID_HEADER_VERSION] = -EBADMSG,
+	[EC_RES_INVALID_HEADER_CRC] = -EBADMSG,
+	[EC_RES_INVALID_DATA_CRC] = -EBADMSG,
+	[EC_RES_DUP_UNAVAILABLE] = -ENODATA,
+};
+
+static int cros_ec_map_error(uint32_t result)
+{
+	int ret = 0;
+
+	if (result != EC_RES_SUCCESS) {
+		if (result < ARRAY_SIZE(cros_ec_error_map) && cros_ec_error_map[result])
+			ret = cros_ec_error_map[result];
+		else
+			ret = -EPROTO;
+	}
+
+	return ret;
+}
+
 static int prepare_packet(struct cros_ec_device *ec_dev,
 			  struct cros_ec_command *msg)
 {
@@ -579,26 +616,24 @@ static int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
  *
  * Return:
  * >=0 - The number of bytes transferred
- * -ENOPROTOOPT - Operation not supported
- * -EPROTO - Protocol error
+ * <0 - Linux error code
  */
 int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
 			    struct cros_ec_command *msg)
 {
-	int ret;
+	int ret, mapped;
 
 	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_INVALID_VERSION) {
-		dev_dbg(ec_dev->dev, "Command invalid version (err:%d)\n",
-			msg->result);
-		return -ENOPROTOOPT;
-	} else if (msg->result != EC_RES_SUCCESS) {
-		dev_dbg(ec_dev->dev, "Command result (err: %d)\n", msg->result);
-		return -EPROTO;
+		return ret;
+	}
+	mapped = cros_ec_map_error(msg->result);
+	if (mapped) {
+		dev_dbg(ec_dev->dev, "Command result (err: %d [%d])\n",
+			msg->result, mapped);
+		ret = mapped;
 	}
-
 	return ret;
 }
 EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
-- 
2.17.1


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

* [PATCH v5 7/7] pwm: cros-ec: Simplify EC error handling
  2020-08-22 15:08 [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Guenter Roeck
                   ` (5 preceding siblings ...)
  2020-08-22 15:08 ` [PATCH v5 6/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Guenter Roeck
@ 2020-08-22 15:08 ` Guenter Roeck
  2020-09-01  9:10 ` [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Enric Balletbo i Serra
  7 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2020-08-22 15:08 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Jonathan Cameron, Benson Leung, Dmitry Torokhov, Thierry Reding,
	Uwe Kleine-König, Lee Jones, Gwendal Grignou, Yu-Hsuan Hsu,
	Prashant Malani, linux-iio, linux-input, linux-pwm, linux-kernel,
	Guenter Roeck

With enhanced error reporting from cros_ec_cmd_xfer_status() in place,
we can fully use it and no longer rely on EC error codes.

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: Added Brian's Reviewed-by: tag
    Added Uwe's Acked-by: tag
v4: Added patch to series

 drivers/pwm/pwm-cros-ec.c | 26 ++++----------------------
 1 file changed, 4 insertions(+), 22 deletions(-)

diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
index 94d3dff9b0e5..c1c337969e4e 100644
--- a/drivers/pwm/pwm-cros-ec.c
+++ b/drivers/pwm/pwm-cros-ec.c
@@ -81,8 +81,7 @@ static int cros_ec_pwm_set_duty(struct cros_ec_device *ec, u8 index, u16 duty)
 	return cros_ec_cmd_xfer_status(ec, msg);
 }
 
-static int __cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index,
-				  u32 *result)
+static int cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index)
 {
 	struct {
 		struct cros_ec_command msg;
@@ -107,19 +106,12 @@ static int __cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index,
 	params->index = index;
 
 	ret = cros_ec_cmd_xfer_status(ec, msg);
-	if (result)
-		*result = msg->result;
 	if (ret < 0)
 		return ret;
 
 	return resp->duty;
 }
 
-static int cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index)
-{
-	return __cros_ec_pwm_get_duty(ec, index, NULL);
-}
-
 static int cros_ec_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			     const struct pwm_state *state)
 {
@@ -215,28 +207,18 @@ static int cros_ec_num_pwms(struct cros_ec_device *ec)
 
 	/* The index field is only 8 bits */
 	for (i = 0; i <= U8_MAX; i++) {
-		u32 result = 0;
-
-		ret = __cros_ec_pwm_get_duty(ec, i, &result);
+		ret = cros_ec_pwm_get_duty(ec, i);
 		/*
 		 * We look for SUCCESS, INVALID_COMMAND, or INVALID_PARAM
 		 * responses; everything else is treated as an error.
-		 * The EC error codes either map to -EOPNOTSUPP / -EINVAL,
-		 * or -EPROTO is returned and the EC error is in the result
-		 * field. Check for both.
+		 * The EC error codes map to -EOPNOTSUPP and -EINVAL,
+		 * so check for those.
 		 */
 		switch (ret) {
 		case -EOPNOTSUPP:	/* invalid command */
 			return -ENODEV;
 		case -EINVAL:		/* invalid parameter */
 			return i;
-		case -EPROTO:
-			/* Old or new error return code: Handle both */
-			if (result == EC_RES_INVALID_COMMAND)
-				return -ENODEV;
-			else if (result == EC_RES_INVALID_PARAM)
-				return i;
-			return -EPROTO;
 		default:
 			if (ret < 0)
 				return ret;
-- 
2.17.1


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

* Re: [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes
  2020-08-22 15:08 [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Guenter Roeck
                   ` (6 preceding siblings ...)
  2020-08-22 15:08 ` [PATCH v5 7/7] pwm: cros-ec: Simplify EC error handling Guenter Roeck
@ 2020-09-01  9:10 ` Enric Balletbo i Serra
  7 siblings, 0 replies; 9+ messages in thread
From: Enric Balletbo i Serra @ 2020-09-01  9:10 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Jonathan Cameron, Benson Leung, Dmitry Torokhov, Thierry Reding,
	Uwe Kleine-König, Lee Jones, Gwendal Grignou, Yu-Hsuan Hsu,
	Prashant Malani, linux-iio, linux-input, linux-pwm, linux-kernel

Dear all,

On 22/8/20 17:08, Guenter Roeck wrote:
> The EC reports a variety of error codes. Most of those, with the exception
> of EC_RES_INVALID_VERSION, are converted to -EPROTO. As result, the actual
> error code gets lost. In cros_ec_cmd_xfer_status(), convert all EC errors
> to Linux error codes to report a more meaningful error to the caller to aid
> debugging.
> 
> To prepare for this change, handle error codes other than -EPROTO for all
> callers of cros_ec_cmd_xfer_status(). Specifically, no longer assume that
> -EPROTO reflects an error from the EC and all other error codes reflect a
> transfer error.
> 
> v2: Add patches 1/4 to 3/4 to handle callers of cros_ec_cmd_xfer_status()
> v3: Add patches 4/6 and 5/6 to handle additional callers of
> 	cros_ec_cmd_xfer_status()
>     Use -ENOPROTOOPT for EC_RES_INVALID_VERSION
>     Implement function to convert error codes
> v4: Add coments describing the functionality of cros_ec_num_pwms().
>     Add patch 7/7 to clean up cros_ec_num_pwms() after the new error code
>     support has been implemented.
>     Rebased series to v5.8.
> v5: If there is no error, cros_ec_cmd_xfer_status() needs to return the
>     number of received bytes, not 0. While fixing that (in patch 6/7),
>     rearranged the function to reduce its indentation.
>     Rebased series to v5.9-rc1
> 
> ----------------------------------------------------------------
> Guenter Roeck (7):
>       iio: cros_ec: Accept -EOPNOTSUPP as 'not supported' error code
>       cros_ec_lightbar: Accept more error codes from cros_ec_cmd_xfer_status
>       platform/chrome: cros_ec_sysfs: Report range of error codes from EC
>       pwm: cros-ec: Accept more error codes from cros_ec_cmd_xfer_status
>       platform/input: cros_ec: Replace -ENOTSUPP with -ENOPROTOOPT
>       platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes
>       pwm: cros-ec: Simplify EC error handling
> 
>  .../iio/common/cros_ec_sensors/cros_ec_sensors.c   |  2 +-
>  drivers/input/keyboard/cros_ec_keyb.c              |  2 +-
>  drivers/platform/chrome/cros_ec_lightbar.c         | 10 ++--
>  drivers/platform/chrome/cros_ec_proto.c            | 57 +++++++++++++++++-----
>  drivers/platform/chrome/cros_ec_sysfs.c            | 24 ++++-----
>  drivers/pwm/pwm-cros-ec.c                          | 37 +++++++-------
>  6 files changed, 78 insertions(+), 54 deletions(-)
> 

I applied all the patches and queued for 5.10 in the chrome-platform tree. If at
some point we need to create an immutable branch I can provide it.

Thanks,
 Enric

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

end of thread, other threads:[~2020-09-01  9:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-22 15:08 [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Guenter Roeck
2020-08-22 15:08 ` [PATCH v5 1/7] iio: cros_ec: Accept -EOPNOTSUPP as 'not supported' error code Guenter Roeck
2020-08-22 15:08 ` [PATCH v5 2/7] cros_ec_lightbar: Accept more error codes from cros_ec_cmd_xfer_status Guenter Roeck
2020-08-22 15:08 ` [PATCH v5 3/7] platform/chrome: cros_ec_sysfs: Report range of error codes from EC Guenter Roeck
2020-08-22 15:08 ` [PATCH v5 4/7] pwm: cros-ec: Accept more error codes from cros_ec_cmd_xfer_status Guenter Roeck
2020-08-22 15:08 ` [PATCH v5 5/7] platform/input: cros_ec: Replace -ENOTSUPP with -ENOPROTOOPT Guenter Roeck
2020-08-22 15:08 ` [PATCH v5 6/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Guenter Roeck
2020-08-22 15:08 ` [PATCH v5 7/7] pwm: cros-ec: Simplify EC error handling Guenter Roeck
2020-09-01  9:10 ` [PATCH v5 0/7] platform/chrome: cros_ec_proto: Convert EC error codes to Linux error codes Enric Balletbo i Serra

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).