On Thu, Mar 21, 2019 at 8:15 AM Guenter Roeck <gro...@google.com> wrote: > > On Wed, Mar 20, 2019 at 2:21 PM Evan Green <evgr...@chromium.org> wrote: > > > > Add support in code for the new forms of the host sleep event. > > Detects the presence of this version of the command at runtime, > > and use whichever form the EC supports. At this time, always > > request the default timeout, and only report the failing response > > via a WARN(). Future versions could accept the sleep parameter > > from outside the driver, and return the response information to > > usermode or elsewhere. > > > > Signed-off-by: Evan Green <evgr...@chromium.org> > > > > --- > > > > drivers/mfd/cros_ec.c | 37 +++++++++++++++++++++---- > > drivers/platform/chrome/cros_ec_proto.c | 9 ++++++ > > include/linux/mfd/cros_ec.h | 2 ++ > > 3 files changed, 43 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c > > index 6acfe036d522..5305ea706aa9 100644 > > --- a/drivers/mfd/cros_ec.c > > +++ b/drivers/mfd/cros_ec.c > > @@ -75,20 +75,47 @@ static irqreturn_t ec_irq_thread(int irq, void *data) > > > > static int cros_ec_sleep_event(struct cros_ec_device *ec_dev, u8 > > sleep_event) > > { > > + int ret; > > struct { > > struct cros_ec_command msg; > > - struct ec_params_host_sleep_event req; > > + union { > > + struct ec_params_host_sleep_event req0; > > + struct ec_params_host_sleep_event_v1 req1; > > + struct ec_response_host_sleep_event_v1 resp1; > > + } u; > > } __packed buf; > > > > memset(&buf, 0, sizeof(buf)); > > > > - buf.req.sleep_event = sleep_event; > > + if (ec_dev->host_sleep_v1) { > > + buf.u.req1.sleep_event = sleep_event; > > + buf.u.req1.u.suspend_params.sleep_timeout_ms = > > + EC_HOST_SLEEP_TIMEOUT_DEFAULT; > > + > > + buf.msg.outsize = sizeof(buf.u.req1); > > + buf.msg.insize = sizeof(buf.u.resp1); > > + buf.msg.version = 1; > > + > > + } else { > > + buf.u.req0.sleep_event = sleep_event; > > + buf.msg.outsize = sizeof(buf.u.req0); > > + buf.msg.version = 0; > > Unnecessary assignment (already 0).
Ok. > > + } > > > > buf.msg.command = EC_CMD_HOST_SLEEP_EVENT; > > - buf.msg.version = 0; > > - buf.msg.outsize = sizeof(buf.req); > > > > - return cros_ec_cmd_xfer(ec_dev, &buf.msg); > > + ret = cros_ec_cmd_xfer(ec_dev, &buf.msg); > > + > > + /* For now, report failure to transition to S0ix with a warning. */ > > + if (ret >= 0 && ec_dev->host_sleep_v1 && > > + (sleep_event == HOST_SLEEP_EVENT_S0IX_RESUME)) > > + WARN(buf.u.resp1.u.resume_response.sleep_transitions & > > Is WARN really appropriate here, or would dev_warn() do ? I think we want to be pretty loud here. This represents a failure to enter suspend as detected by the EC. > > > + EC_HOST_RESUME_SLEEP_TIMEOUT, > > + "EC detected sleep transition timeout. Total slp_s0 > > transitions: %d", > > + buf.u.resp1.u.resume_response.sleep_transitions & > > + EC_HOST_RESUME_SLEEP_TRANSITIONS_MASK); > > + > > + return ret; > > } > > > > int cros_ec_register(struct cros_ec_device *ec_dev) > > diff --git a/drivers/platform/chrome/cros_ec_proto.c > > b/drivers/platform/chrome/cros_ec_proto.c > > index 97a068dff192..78ceab659a36 100644 > > --- a/drivers/platform/chrome/cros_ec_proto.c > > +++ b/drivers/platform/chrome/cros_ec_proto.c > > @@ -414,6 +414,15 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev) > > else > > ec_dev->mkbp_event_supported = 1; > > > > + /* Probe if host sleep v1 is supported for S0ix failure detection. > > */ > > + ret = cros_ec_get_host_command_version_mask(ec_dev, > > + EC_CMD_HOST_SLEEP_EVENT, > > + &ver_mask); > > + if (ret < 0 || !(ver_mask & EC_VER_MASK(1))) > > + ec_dev->host_sleep_v1 = 0; > > + else > > + ec_dev->host_sleep_v1 = 1; > > This is a boolean. > ec_dev->host_sleep_v1 = (ret >= 0 && (ver_mask & EC_VER_MASK(1))); Ok.