Messages in this thread | | | From | Prashant Malani <> | Date | Thu, 14 Jul 2022 16:33:19 -0700 | Subject | Re: [PATCH v2] platform/chrome: cros_ec: Send host event for prepare/complete |
| |
On Thu, Jul 14, 2022 at 3:17 PM Tim Van Patten <timvp@google.com> wrote: > > Hi, > > > Calling such a theoretical new EC host command from the userspace power manager > > would probably accomplish the same thing. > > We investigated something like that internally initially: > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3646238) > > We decided to take this simpler approach instead, since it > accomplishes the same goal without requiring new host commands.
- That link discusses updating the existing host command and the EC implementations; have you investigated adding a command for logging needs and calling it from power_manager/powerd directly? - That link addresses several busses (UART, SPI etc.) whereas this patch only touches LPC. Is this intentional?
> > > From the kernel documentation[2], "The prepare phase is meant to > > prevent races by preventing new devices from being registered; the PM core > > would never know that all the children of a device had been suspended if new > > children could be registered at will." and "The method may also prepare the > > device or driver in some way for the upcoming system power transition, but > > it should not put the device into a low-power state." > > > > So it seems like an incorrect usage of this callback. > > Why is this usage incorrect? > > Sending the message to the EC is the preparation for the AP to enter > the system power transition. For example, it allows the EC to begin > watchdogging the AP once the suspend begins and stop once the resume > completes. This allows the EC to watchdog the entire process, without > any gaps - a beneficial side effect of this change.
OK. Is that all it does? Does this not allow the EC itself to enter a deep sleep state, or put connected peripherals and/or state machines in a low power state??
If you're still set on this approach, please update the function names; setting the .prepare()/.complete() function pointers to cros_ec_lpc_suspend/resume() is inconsistent, if those functions aren't being used elsewhere in the file.
BR,
-Prashant
| |