On Wed, 25 Jan 2017, Thierry Escande wrote: > From: Shawn Nematbakhsh <sha...@chromium.org> > > The subset of wake-enabled host events is defined by the EC, but the EC > may still send non-wake host events if we're in the process of > suspending. Get the mask of wake-enabled host events from the EC and > filter out non-wake events to prevent spurious aborted suspend > attempts. > > Signed-off-by: Shawn Nematbakhsh <sha...@chromium.org> > Signed-off-by: Thierry Escande <thierry.esca...@collabora.com> > --- > drivers/mfd/cros_ec.c | 14 ++++++-- > drivers/platform/chrome/cros_ec_proto.c | 60 > +++++++++++++++++++++++++++++++++ > include/linux/mfd/cros_ec.h | 12 +++++++ > 3 files changed, 84 insertions(+), 2 deletions(-) > > diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c > index abd8342..510dfbb 100644 > --- a/drivers/mfd/cros_ec.c > +++ b/drivers/mfd/cros_ec.c > @@ -53,12 +53,22 @@ static const struct mfd_cell ec_pd_cell = { > static irqreturn_t ec_irq_thread(int irq, void *data) > { > struct cros_ec_device *ec_dev = data; > + u8 wake_event = 1;
Bool, true? > + u32 host_event; > int ret; > > - if (device_may_wakeup(ec_dev->dev)) > + ret = cros_ec_get_next_event(ec_dev); What does (ret == 0) mean? ... and is is possible for (ret < 0)? > + if (ret > 0 && ec_dev->mkbp_event_supported) { cros_ec_get_host_event() checks for (ec_dev->mkbp_event_supported) anyway, so you can drop it here no? > + /* Don't signal wake event for non-wake host events */ > + host_event = cros_ec_get_host_event(ec_dev); > + if (host_event && !(host_event & ec_dev->host_event_wake_mask)) > + wake_event = 0; false > + } > + > + if (wake_event && device_may_wakeup(ec_dev->dev)) > pm_wakeup_event(ec_dev->dev, 0); > > - ret = cros_ec_get_next_event(ec_dev); > if (ret > 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 256249b..a216a32 100644 > --- a/drivers/platform/chrome/cros_ec_proto.c > +++ b/drivers/platform/chrome/cros_ec_proto.c > @@ -150,6 +150,40 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev, > } > EXPORT_SYMBOL(cros_ec_check_result); > > +/* > + * cros_ec_get_host_event_wake_mask > + * > + * Get the mask of host events that cause wake from suspend. > + * > + * @ec_dev: EC device to call > + * @msg: message structure to use > + * @mask: result when function returns >=0. > + * > + * LOCKING: > + * the caller has ec_dev->lock mutex, or the caller knows there is > + * no other command in progress. > + */ > +static int cros_ec_get_host_event_wake_mask(struct cros_ec_device *ec_dev, > + struct cros_ec_command *msg, > + uint32_t *mask) > +{ > + struct ec_response_host_event_mask *r; > + int ret; > + > + msg->command = EC_CMD_HOST_EVENT_GET_WAKE_MASK; > + msg->version = 0; > + msg->outsize = 0; > + msg->insize = sizeof(*r); > + > + ret = send_command(ec_dev, msg); > + if (ret > 0) { > + r = (struct ec_response_host_event_mask *)msg->data; > + *mask = r->mask; > + } > + > + return ret; > +} > + > static int cros_ec_host_command_proto_query(struct cros_ec_device *ec_dev, > int devidx, > struct cros_ec_command *msg) > @@ -387,6 +421,15 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev) > else > ec_dev->mkbp_event_supported = 1; > > + /* > + * Get host event wake mask, assume all events are wake events > + * if unavailable. > + */ > + ret = cros_ec_get_host_event_wake_mask(ec_dev, proto_msg, > + &ec_dev->host_event_wake_mask); > + if (ret < 0) > + ec_dev->host_event_wake_mask = U32_MAX; > + > ret = 0; > > exit: > @@ -507,3 +550,20 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev) > return get_keyboard_state_event(ec_dev); > } > EXPORT_SYMBOL(cros_ec_get_next_event); > + > +u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev) > +{ > + if (WARN_ON(!ec_dev->mkbp_event_supported)) > + return 0; > + > + if (ec_dev->event_data.event_type != EC_MKBP_EVENT_HOST_EVENT) > + return 0; > + > + if (ec_dev->event_size != sizeof(u32)) { > + dev_warn(ec_dev->dev, "Invalid host event size\n"); > + return 0; > + } > + > + return get_unaligned_le32(&ec_dev->event_data.data.host_event); > +} > +EXPORT_SYMBOL(cros_ec_get_host_event); > diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h > index f62043a..8f37c4e 100644 > --- a/include/linux/mfd/cros_ec.h > +++ b/include/linux/mfd/cros_ec.h > @@ -146,6 +146,7 @@ struct cros_ec_device { > > struct ec_response_get_next_event event_data; > int event_size; > + u32 host_event_wake_mask; > }; > > /** > @@ -297,6 +298,17 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev); > */ > int cros_ec_get_next_event(struct cros_ec_device *ec_dev); > > +/** > + * cros_ec_get_host_event - Return a mask of event set by the EC. > + * > + * Once cros_ec_get_next_event() has been called, if the event source is > + * a host event, this function returns the precise event that triggered > + * the interrupt. > + * > + * This function is a helper to know which events are raised. > + */ > +u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev); > + > /* sysfs stuff */ > extern struct attribute_group cros_ec_attr_group; > extern struct attribute_group cros_ec_lightbar_attr_group; -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog