linux-kernel - Re: [PATCH] [v9] platform/chrome: cros_ec_lpc: Move host command to prepare/complete

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZGQ9Y+vqWhQLHAQh@google.com>
Date:   Wed, 17 May 2023 10:35:15 +0800
From:   Tzung-Bi Shih <tzungbi@...nel.org>
To:     Tim Van Patten <timvp@...omium.org>
Cc:     LKML <linux-kernel@...r.kernel.org>, robbarnes@...gle.com,
        lalithkraj@...gle.com, rrangel@...omium.org,
        Benson Leung <bleung@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        chrome-platform@...ts.linux.dev
Subject: Re: [PATCH] [v9] platform/chrome: cros_ec_lpc: Move host command to
 prepare/complete

On Mon, May 15, 2023 at 02:25:52PM -0600, Tim Van Patten wrote:
> Update cros_ec_lpc_pm_ops to call cros_ec_lpc_prepare() during PM
> .prepare() and cros_ec_lpc_complete() during .complete(). This moves the
> host command that the AP sends and allows the EC to log entry/exit of
> AP's suspend/resume more accurately.

I can understand the patch wants to notify EC earlier/later when the system
suspend/resume.  But what is the issue addressed?  What happens if the
measurement of suspend/resume duration is not that accurate?

Copied from my previous mail:
* Should it move the callbacks?
  * Is it appropriate to call cros_ec_suspend() when PM is still in prepare
    phase and call cros_ec_resume() when PM is already in complete phase?

It seems prepare() is a more general callback.  It could be followed by
suspend(), freeze(), or poweroff()[1].  Do we expect the change?  For example,
the system is going to power off but EC gets notification about the system
should be going to suspend.  Same as complete().

Moreover, cros_ec_suspend() and cros_ec_resume() do more than just notify EC.
E.g. [2].

What about other interfaces (i2c, spi, uart)?  Do they also need to change
the callbacks?

[1]: https://elixir.bootlin.com/linux/v6.4-rc1/source/include/linux/pm.h#L74
[2]: https://elixir.bootlin.com/linux/v6.4-rc1/source/drivers/platform/chrome/cros_ec.c#L351

> Changes in v9:
> - Remove log statements.
> - Ignore return value from cros_ec_resume().

The change logs are not part of commit message.  They should put after "---".