On Wed, Nov 7, 2018 at 5:30 PM Brian Norris <[email protected]> wrote: > > 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').
Hmm, I realized there's a problem with this: I'd need to change get_keyboard_state_event() too, if I want to do this right. I think maybe it'll be better to leave this alone, and just fix up the documentation and a few corner cases. > 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 <[email protected]> > --- > This barely touches MFD code, but overall this series should probably go > through the platform/chrome maintenance, IMO. > --- > drivers/mfd/cros_ec.c | 4 ++-- I may end up dropping the MFD changes from the next version, so I might drop Lee from the list too. Everyone else...stay tuned for v2. Thanks, Brian > 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 >
