lkml.org 
[lkml]   [2022]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] platform/chrome: cros_ec: Always expose last resume result
On Fri, Jun 10, 2022 at 3:37 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> The last resume result exposing logic in cros_ec_sleep_event()
> incorrectly requires S0ix support, which doesn't work on ARM based
> systems where S0ix doesn't exist. That's because cros_ec_sleep_event()
> only reports the last resume result when the EC indicates the last sleep
> event was an S0ix resume. On ARM systems, the last sleep event is always
> S3 resume, but the EC can still detect sleep hang events in case some
> other part of the AP is blocking sleep.
>
> Always expose the last resume result if the EC supports it so that this
> works on all devices regardless of S0ix support. This fixes sleep hang
> detection on ARM based chromebooks like Trogdor.
>
> Cc: Evan Green <evgreen@chromium.org>
> Cc: Rajat Jain <rajatja@chromium.org>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> Fixes: 7235560ac77a ("platform/chrome: Add support for v1 of host sleep event")
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>
> I originally was going to check for S3 only with !ARCH_X86 but I can't
> convince myself that it's any use to limit the check to s0ix in general.
> This approach assumes the last resume result is valid if the command is
> supported, regardless of whether or not the AP supports s0ix or not.
> This seems to be the case at least on ARM, and looking at the EC
> convinces me that CONFIG_POWER_SLEEP_FAILURE_DETECTION is only enabled
> on x86 devices that have s0ix.
>
> drivers/platform/chrome/cros_ec.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> index b3e94cdf7d1a..881d3ad09be0 100644
> --- a/drivers/platform/chrome/cros_ec.c
> +++ b/drivers/platform/chrome/cros_ec.c
> @@ -135,10 +135,10 @@ static int cros_ec_sleep_event(struct cros_ec_device *ec_dev, u8 sleep_event)
> buf.msg.command = EC_CMD_HOST_SLEEP_EVENT;
>
> ret = cros_ec_cmd_xfer_status(ec_dev, &buf.msg);
> -
> - /* For now, report failure to transition to S0ix with a warning. */
> + /* Report failure to transition to system wide suspend with a warning. */
> if (ret >= 0 && ec_dev->host_sleep_v1 &&
> - (sleep_event == HOST_SLEEP_EVENT_S0IX_RESUME)) {
> + (sleep_event == HOST_SLEEP_EVENT_S0IX_SUSPEND ||
> + sleep_event == HOST_SLEEP_EVENT_S3_RESUME)) {

I am sure I am missing something, but the description doesn't explain
(to me) the switch from HOST_SLEEP_EVENT_S0IX_RESUME to
HOST_SLEEP_EVENT_S0IX_SUSPEND.

Thanks,
Guenter

> ec_dev->last_resume_result =
> buf.u.resp1.resume_response.sleep_transitions;
>
>
> base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56
> --
> https://chromeos.dev
>

\
 
 \ /
  Last update: 2022-06-11 03:41    [W:0.057 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site