[PATCH 0/2] platform/chrome: Add support for host sleep event command v1

[PATCH 0/2] platform/chrome: Add support for host sleep event command v1

[Evan Green]

The Chrome OS EC has an updated set of parameters for the host
sleep event command. With the new parameters, the host can indicate
a timeout along with suspend messages. Specifically S0ix suspend
messages are supported now, though the host command format isn't
specific to S0ix. When the EC sees an S0ix suspend host sleep event,
it arms a timer for the specified number of milliseconds (or a sane
per-board default baked into the EC). If the EC does not observe
the platform's SLP_S0 line assert within the specified timeout, then
the EC wakes the system.

On resume, the EC reports the number of transitions seen on the SLP_S0
line. The high bit is used to report whether or not a timeout occurred.
The number of transitions can then be used to detect cases of excessive
housekeeping activities, where the system wakes up out of S0ix temporarily
(unbeknownst to Linux), and then (hopefully) goes back to sleep.

This mechanism helps in cases where the system attempted to suspend
via S0ix, but due to driver bugs ended up suspending to a shallower
idle state instead. In concert with additional changes that detect
S0ix entry failures, this mechanism allows the system to quickly
detect and report on incorrect suspend outcomes.


Evan Green (2):
  mfd: cros_ec: Add host_sleep_event_v1 command
  platform/chrome: Add support for v1 of host sleep event

 drivers/mfd/cros_ec.c                   | 37 +++++++++++++---
 drivers/platform/chrome/cros_ec_proto.c |  9 ++++
 include/linux/mfd/cros_ec.h             |  2 +
 include/linux/mfd/cros_ec_commands.h    | 59 +++++++++++++++++++++++++
 4 files changed, 102 insertions(+), 5 deletions(-)

-- 
2.20.1

[PATCH 1/2] mfd: cros_ec: Add host_sleep_event_v1 command

[Evan Green]

Introduce the command and response structures for the second revision
of the host sleep event. These structures are part of a new EC change
that enables detection of failure to enter S0ix. The EC waits a
kernel-specified timeout (or a default amount of time) for the S0_SLP
pin to change, and wakes the system if that change does not occur in
time.

Signed-off-by: Evan Green <evgreen@chromium.org>

---

 include/linux/mfd/cros_ec_commands.h | 59 ++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index fc91082d4c35..4db1240c28a8 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -2729,6 +2729,65 @@ struct ec_params_host_sleep_event {
 	uint8_t sleep_event;
 } __packed;
 
+/*
+ * Use a default timeout value (CONFIG_SLEEP_TIMEOUT_MS) for detecting sleep
+ * transition failures
+ */
+#define EC_HOST_SLEEP_TIMEOUT_DEFAULT 0
+
+/* Disable timeout detection for this sleep transition */
+#define EC_HOST_SLEEP_TIMEOUT_INFINITE 0xFFFF
+
+struct ec_params_host_sleep_event_v1 {
+	/* The type of sleep being entered or exited. */
+	uint8_t sleep_event;
+
+	/* Padding */
+	uint8_t reserved;
+	union {
+		/* Parameters that apply for suspend messages. */
+		struct {
+			/*
+			 * The timeout in milliseconds between when this message
+			 * is received and when the EC will declare sleep
+			 * transition failure if the sleep signal is not
+			 * asserted.
+			 */
+			uint16_t sleep_timeout_ms;
+		} suspend_params;
+
+		/* Reserved space for non-suspend commands. */
+		uint16_t reserved;
+	} u;
+} __packed;
+
+/* A timeout occurred when this bit is set */
+#define EC_HOST_RESUME_SLEEP_TIMEOUT 0x80000000
+
+/*
+ * The mask defining which bits correspond to the number of sleep transitions,
+ * as well as the maximum number of suspend line transitions that will be
+ * reported back to the host.
+ */
+#define EC_HOST_RESUME_SLEEP_TRANSITIONS_MASK 0x7FFFFFFF
+
+struct ec_response_host_sleep_event_v1 {
+	union {
+		/* Response fields that apply for resume messages. */
+		struct {
+			/*
+			 * The number of sleep power signal transitions that
+			 * occurred since the suspend message. The high bit
+			 * indicates a timeout occurred.
+			 */
+			uint32_t sleep_transitions;
+		} resume_response;
+
+		/* Padding for non-resume responses. */
+		uint32_t reserved;
+	} u;
+} __packed;
+
 /*****************************************************************************/
 /* Smart battery pass-through */
 
-- 
2.20.1

[PATCH 2/2] platform/chrome: Add support for v1 of host sleep event

[Evan Green]

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 <evgreen@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;
+	}
 
 	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 &
+		     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;
+
 	/*
 	 * Get host event wake mask, assume all events are wake events
 	 * if unavailable.
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 8f2a8918bfa3..b6442201f77f 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -120,6 +120,7 @@ struct cros_ec_command {
  * @pkt_xfer: Send packet to EC and get response.
  * @lock: One transaction at a time.
  * @mkbp_event_supported: True if this EC supports the MKBP event protocol.
+ * @host_sleep_v1: True if this EC supports the sleep v1 command.
  * @event_notifier: Interrupt event notifier for transport devices.
  * @event_data: Raw payload transferred with the MKBP event.
  * @event_size: Size in bytes of the event data.
@@ -153,6 +154,7 @@ struct cros_ec_device {
 			struct cros_ec_command *msg);
 	struct mutex lock;
 	bool mkbp_event_supported;
+	bool host_sleep_v1;
 	struct blocking_notifier_head event_notifier;
 
 	struct ec_response_get_next_event_v1 event_data;
-- 
2.20.1

Re: [PATCH 2/2] platform/chrome: Add support for v1 of host sleep event

[Guenter Roeck]

On Wed, Mar 20, 2019 at 2:21 PM Evan Green <evgreen@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 <evgreen@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).
> +       }
>
>         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 ?

> +                    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)));

> +
>         /*
>          * Get host event wake mask, assume all events are wake events
>          * if unavailable.
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 8f2a8918bfa3..b6442201f77f 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -120,6 +120,7 @@ struct cros_ec_command {
>   * @pkt_xfer: Send packet to EC and get response.
>   * @lock: One transaction at a time.
>   * @mkbp_event_supported: True if this EC supports the MKBP event protocol.
> + * @host_sleep_v1: True if this EC supports the sleep v1 command.
>   * @event_notifier: Interrupt event notifier for transport devices.
>   * @event_data: Raw payload transferred with the MKBP event.
>   * @event_size: Size in bytes of the event data.
> @@ -153,6 +154,7 @@ struct cros_ec_device {
>                         struct cros_ec_command *msg);
>         struct mutex lock;
>         bool mkbp_event_supported;
> +       bool host_sleep_v1;
>         struct blocking_notifier_head event_notifier;
>
>         struct ec_response_get_next_event_v1 event_data;
> --
> 2.20.1
>

Re: [PATCH 1/2] mfd: cros_ec: Add host_sleep_event_v1 command

[Enric Balletbo Serra]

Hi Evan,Thanks for sending this upstream, one comment below.
Missatge de Evan Green <evgreen@chromium.org> del dia dc., 20 de març
2019 a les 22:24:
>
> Introduce the command and response structures for the second revision
> of the host sleep event. These structures are part of a new EC change
> that enables detection of failure to enter S0ix. The EC waits a
> kernel-specified timeout (or a default amount of time) for the S0_SLP
> pin to change, and wakes the system if that change does not occur in
> time.
>
> Signed-off-by: Evan Green <evgreen@chromium.org>
>
> ---
>
>  include/linux/mfd/cros_ec_commands.h | 59 ++++++++++++++++++++++++++++

We're trying to sync kernel cros_ec_commands.h with the EC protocol at
https://chromium.googlesource.com/chromiumos/platform/ec/+/master/include/ec_commands.h

Gwendal send a first patch [1] and a second version will be sent soon.
I don't see these definitions in the mentioned patch neither the
master ec_commands.h from the EC firmware repository. Is this a
feature that didn't land in the EC firmware yet?

[1] https://lkml.org/lkml/2019/2/27/723

Thanks,
 Enric

>  1 file changed, 59 insertions(+)
>
> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
> index fc91082d4c35..4db1240c28a8 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -2729,6 +2729,65 @@ struct ec_params_host_sleep_event {
>         uint8_t sleep_event;
>  } __packed;
>
> +/*
> + * Use a default timeout value (CONFIG_SLEEP_TIMEOUT_MS) for detecting sleep
> + * transition failures
> + */
> +#define EC_HOST_SLEEP_TIMEOUT_DEFAULT 0
> +
> +/* Disable timeout detection for this sleep transition */
> +#define EC_HOST_SLEEP_TIMEOUT_INFINITE 0xFFFF
> +
> +struct ec_params_host_sleep_event_v1 {
> +       /* The type of sleep being entered or exited. */
> +       uint8_t sleep_event;
> +
> +       /* Padding */
> +       uint8_t reserved;
> +       union {
> +               /* Parameters that apply for suspend messages. */
> +               struct {
> +                       /*
> +                        * The timeout in milliseconds between when this message
> +                        * is received and when the EC will declare sleep
> +                        * transition failure if the sleep signal is not
> +                        * asserted.
> +                        */
> +                       uint16_t sleep_timeout_ms;
> +               } suspend_params;
> +
> +               /* Reserved space for non-suspend commands. */
> +               uint16_t reserved;
> +       } u;
> +} __packed;
> +
> +/* A timeout occurred when this bit is set */
> +#define EC_HOST_RESUME_SLEEP_TIMEOUT 0x80000000
> +
> +/*
> + * The mask defining which bits correspond to the number of sleep transitions,
> + * as well as the maximum number of suspend line transitions that will be
> + * reported back to the host.
> + */
> +#define EC_HOST_RESUME_SLEEP_TRANSITIONS_MASK 0x7FFFFFFF
> +
> +struct ec_response_host_sleep_event_v1 {
> +       union {
> +               /* Response fields that apply for resume messages. */
> +               struct {
> +                       /*
> +                        * The number of sleep power signal transitions that
> +                        * occurred since the suspend message. The high bit
> +                        * indicates a timeout occurred.
> +                        */
> +                       uint32_t sleep_transitions;
> +               } resume_response;
> +
> +               /* Padding for non-resume responses. */
> +               uint32_t reserved;
> +       } u;
> +} __packed;
> +
>  /*****************************************************************************/
>  /* Smart battery pass-through */
>
> --
> 2.20.1
>

Re: [PATCH 1/2] mfd: cros_ec: Add host_sleep_event_v1 command

[Evan Green]

On Thu, Mar 21, 2019 at 2:31 PM Enric Balletbo Serra
<eballetbo@gmail.com> wrote:
>
> Hi Evan,Thanks for sending this upstream, one comment below.
> Missatge de Evan Green <evgreen@chromium.org> del dia dc., 20 de març
> 2019 a les 22:24:
> >
> > Introduce the command and response structures for the second revision
> > of the host sleep event. These structures are part of a new EC change
> > that enables detection of failure to enter S0ix. The EC waits a
> > kernel-specified timeout (or a default amount of time) for the S0_SLP
> > pin to change, and wakes the system if that change does not occur in
> > time.
> >
> > Signed-off-by: Evan Green <evgreen@chromium.org>
> >
> > ---
> >
> >  include/linux/mfd/cros_ec_commands.h | 59 ++++++++++++++++++++++++++++
>
> We're trying to sync kernel cros_ec_commands.h with the EC protocol at
> https://chromium.googlesource.com/chromiumos/platform/ec/+/master/include/ec_commands.h
>
> Gwendal send a first patch [1] and a second version will be sent soon.
> I don't see these definitions in the mentioned patch neither the
> master ec_commands.h from the EC firmware repository. Is this a
> feature that didn't land in the EC firmware yet?
>
> [1] https://lkml.org/lkml/2019/2/27/723
>
> Thanks,
>  Enric

Hi Enric,
That's correct, this feature is hot off the presses. It's being
reviewed here, I expect it to land soon:
https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1501512

It seems like it would be easier if I based this series on top of
Gwendal's. I'll plan to do that for the next spin unless I hear
otherwise.
-Evan


-Evan

Re: [PATCH 1/2] mfd: cros_ec: Add host_sleep_event_v1 command

[Enric Balletbo Serra]

Hi Evan,

Missatge de Evan Green <evgreen@chromium.org> del dia dv., 22 de març
2019 a les 16:38:
>
> On Thu, Mar 21, 2019 at 2:31 PM Enric Balletbo Serra
> <eballetbo@gmail.com> wrote:
> >
> > Hi Evan,Thanks for sending this upstream, one comment below.
> > Missatge de Evan Green <evgreen@chromium.org> del dia dc., 20 de març
> > 2019 a les 22:24:
> > >
> > > Introduce the command and response structures for the second revision
> > > of the host sleep event. These structures are part of a new EC change
> > > that enables detection of failure to enter S0ix. The EC waits a
> > > kernel-specified timeout (or a default amount of time) for the S0_SLP
> > > pin to change, and wakes the system if that change does not occur in
> > > time.
> > >
> > > Signed-off-by: Evan Green <evgreen@chromium.org>
> > >
> > > ---
> > >
> > >  include/linux/mfd/cros_ec_commands.h | 59 ++++++++++++++++++++++++++++
> >
> > We're trying to sync kernel cros_ec_commands.h with the EC protocol at
> > https://chromium.googlesource.com/chromiumos/platform/ec/+/master/include/ec_commands.h
> >
> > Gwendal send a first patch [1] and a second version will be sent soon.
> > I don't see these definitions in the mentioned patch neither the
> > master ec_commands.h from the EC firmware repository. Is this a
> > feature that didn't land in the EC firmware yet?
> >
> > [1] https://lkml.org/lkml/2019/2/27/723
> >
> > Thanks,
> >  Enric
>
> Hi Enric,
> That's correct, this feature is hot off the presses. It's being
> reviewed here, I expect it to land soon:
> https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1501512
>
> It seems like it would be easier if I based this series on top of
> Gwendal's. I'll plan to do that for the next spin unless I hear
> otherwise.

Yes please, if is possible base the series on top of Gwendal's patch.
If for some reason the Gwendal's patch lands before, you can send it
separately but I'd like to see applied in EC repo. The purpose for
that is to maintain sync the includes. Thanks,

Enric

> -Evan
>
>
> -Evan

Re: [PATCH 2/2] platform/chrome: Add support for v1 of host sleep event

[Evan Green]

On Thu, Mar 21, 2019 at 8:15 AM Guenter Roeck <groeck@google.com> wrote:
>
> On Wed, Mar 20, 2019 at 2:21 PM Evan Green <evgreen@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 <evgreen@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.