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.
----------------------------------------------------------------
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 | 52 +++++++++++++++++-----
drivers/platform/chrome/cros_ec_sysfs.c | 24 ++++------
drivers/pwm/pwm-cros-ec.c | 37 +++++++--------
6 files changed, 74 insertions(+), 53 deletions(-)
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 <[email protected]>
Cc: Yu-Hsuan Hsu <[email protected]>
Cc: Prashant Malani <[email protected]>
Cc: Brian Norris <[email protected]>
Reviewed-by: Brian Norris <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
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
-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 <[email protected]>
Cc: Yu-Hsuan Hsu <[email protected]>
Cc: Prashant Malani <[email protected]>
Cc: Brian Norris <[email protected]>
Acked-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
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 3e745e0fe092..e5bbec979a2a 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -555,7 +555,7 @@ EXPORT_SYMBOL(cros_ec_cmd_xfer);
*
* 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,
@@ -569,7 +569,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
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 <[email protected]>
Cc: Yu-Hsuan Hsu <[email protected]>
Cc: Prashant Malani <[email protected]>
Cc: Brian Norris <[email protected]>
Reviewed-by: Brian Norris <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
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
Hi Guenter et all,
On 6/8/20 17:33, 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.
>
> ----------------------------------------------------------------
> 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 | 52 +++++++++++++++++-----
> drivers/platform/chrome/cros_ec_sysfs.c | 24 ++++------
> drivers/pwm/pwm-cros-ec.c | 37 +++++++--------
> 6 files changed, 74 insertions(+), 53 deletions(-)
>
The patches LGTM, and if the other maintainers are fine, I'd like to queue all
these through the chrome-platform tree.
I noticed, thought, that KernelCI reported a regression on Kevin that I'll try
to debug at the beginning of next week.
[ 3.821203] cros-ec-spi spi2.0: Wrong size 1/3: 0 != 4
[ 3.827320] cros-ec-keyb ff200000.spi:ec@0:keyboard-controller: cannot
register non-matrix inputs: -71
[ 3.838506] cros-ec-keyb: probe of ff200000.spi:ec@0:keyboard-controller
failed with error -71
[ 3.853492] cros-ec-spi spi2.0: Chrome EC device registered
Thanks,
Enric
On 8/21/20 1:21 AM, Enric Balletbo i Serra wrote:
> Hi Guenter et all,
>
> On 6/8/20 17:33, 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.
>>
>> ----------------------------------------------------------------
>> 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 | 52 +++++++++++++++++-----
>> drivers/platform/chrome/cros_ec_sysfs.c | 24 ++++------
>> drivers/pwm/pwm-cros-ec.c | 37 +++++++--------
>> 6 files changed, 74 insertions(+), 53 deletions(-)
>>
>
> The patches LGTM, and if the other maintainers are fine, I'd like to queue all
> these through the chrome-platform tree.
>
> I noticed, thought, that KernelCI reported a regression on Kevin that I'll try
> to debug at the beginning of next week.
>
> [ 3.821203] cros-ec-spi spi2.0: Wrong size 1/3: 0 != 4
> [ 3.827320] cros-ec-keyb ff200000.spi:ec@0:keyboard-controller: cannot
> register non-matrix inputs: -71
> [ 3.838506] cros-ec-keyb: probe of ff200000.spi:ec@0:keyboard-controller
> failed with error -71
> [ 3.853492] cros-ec-spi spi2.0: Chrome EC device registered
>
Easy to debug. cros_ec_cmd_xfer_status() now returns 0 for success and < 0 for errors.
It needs to return the receive length if there is no error. I'll send v5 with a fix,
and sorry for the trouble.
Thanks,
Guenter