Messages in this thread Patch in this message | | | From | Brian Norris <> | Subject | [PATCH 1/2] platform/chrome: fixup cros_ec_get_next_event() error codes | Date | Wed, 7 Nov 2018 17:30:04 -0800 |
| |
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
| |