lkml.org 
[lkml]   [2018]   [Nov]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH 1/2] platform/chrome: fixup cros_ec_get_next_event() error codes
Date
cros_ec_get_next_event() is documented to return 0 for success and
negative for errors. It currently returns negative for some errors, and
non-negative (number of bytes received) for success (including some "no
data available" responses as zero). This works out OK, because the
callers were more or less ignoring the documentation, and only treating
positive values as success (and indepdently checking the modification of
'wakeup').

This is difficult to reason about though, and it makes it harder to
refactor cros_ec_get_next_event() without breaking its users. So
instead, I
(a) make cros_ec_get_next_event() match its documentation and
(b) change the users to treat non-negative as success.

This change tries to match a roughly similar pattern to
cros_ec_cmd_xfer_status(), where most non-zero result codes are treated
as an error -- in this case, all protocol errors (including
EC_RES_UNAVAILABLE) are treated as "no data available."

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
This barely touches MFD code, but overall this series should probably go
through the platform/chrome maintenance, IMO.
---
drivers/mfd/cros_ec.c | 4 ++--
drivers/platform/chrome/cros_ec_lpc.c | 2 +-
drivers/platform/chrome/cros_ec_proto.c | 13 ++++++++++---
3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index fe6f83766144..807a5870198d 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -67,7 +67,7 @@ static irqreturn_t ec_irq_thread(int irq, void *data)
if (wake_event && device_may_wakeup(ec_dev->dev))
pm_wakeup_event(ec_dev->dev, 0);

- if (ret > 0)
+ if (ret >= 0)
blocking_notifier_call_chain(&ec_dev->event_notifier,
0, ec_dev);
return IRQ_HANDLED;
@@ -219,7 +219,7 @@ EXPORT_SYMBOL(cros_ec_suspend);
static void cros_ec_report_events_during_suspend(struct cros_ec_device *ec_dev)
{
while (ec_dev->mkbp_event_supported &&
- cros_ec_get_next_event(ec_dev, NULL) > 0)
+ cros_ec_get_next_event(ec_dev, NULL) >= 0)
blocking_notifier_call_chain(&ec_dev->event_notifier,
1, ec_dev);
}
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index e1b75775cd4a..292408eefe43 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -235,7 +235,7 @@ static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data)
struct cros_ec_device *ec_dev = data;

if (ec_dev->mkbp_event_supported &&
- cros_ec_get_next_event(ec_dev, NULL) > 0)
+ cros_ec_get_next_event(ec_dev, NULL) >= 0)
blocking_notifier_call_chain(&ec_dev->event_notifier, 0,
ec_dev);

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index b6fd4838f60f..49625836fdd6 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -541,8 +541,12 @@ static int get_next_event(struct cros_ec_device *ec_dev)
if (cmd_version == 1) {
ret = get_next_event_xfer(ec_dev, msg, cmd_version,
sizeof(struct ec_response_get_next_event_v1));
- if (ret < 0 || msg->result != EC_RES_INVALID_VERSION)
+ if (ret < 0)
return ret;
+ if (msg->result == EC_RES_SUCCESS)
+ return 0;
+ if (msg->result != EC_RES_INVALID_VERSION)
+ return -ENODATA;

/* Fallback to version 0 for future send attempts */
cmd_version = 0;
@@ -550,8 +554,11 @@ static int get_next_event(struct cros_ec_device *ec_dev)

ret = get_next_event_xfer(ec_dev, msg, cmd_version,
sizeof(struct ec_response_get_next_event));
-
- return ret;
+ if (ret < 0)
+ return ret;
+ if (msg->result != EC_RES_SUCCESS)
+ return -ENODATA;
+ return 0;
}

static int get_keyboard_state_event(struct cros_ec_device *ec_dev)
--
2.19.1.930.g4563a0d9d0-goog
\
 
 \ /
  Last update: 2018-11-08 02:30    [W:0.043 / U:0.676 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site