* [PATCH v3] platform/chrome: cros_ec: Send host event for prepare/complete
@ 2022-08-02 17:40 Tim Van Patten
2022-08-17 21:55 ` Raul Rangel
2022-08-18 4:08 ` Tzung-Bi Shih
0 siblings, 2 replies; 6+ messages in thread
From: Tim Van Patten @ 2022-08-02 17:40 UTC (permalink / raw)
To: rrangel, robbarnes
Cc: Tim Van Patten, Benson Leung, Guenter Roeck, chrome-platform,
linux-kernel
Update cros_ec_lpc_pm_ops to call cros_ec_lpc_prepare() during PM
.prepare() and cros_ec_lpc_complete() during .complete(). This allows the
EC to log entry/exit of AP's suspend/resume more accurately.
Signed-off-by: Tim Van Patten <timvp@google.com>
---
Changes in v3:
- Update cros_ec_lpc_suspend() to cros_ec_lpc_prepare()
- Update cros_ec_lpc_resume() to cros_ec_lpc_complete()
Changes in v2:
- Include cros_ec_resume() return value in dev_info() output.
- Guard setting .prepare/.complete with #ifdef CONFIG_PM_SLEEP.
drivers/platform/chrome/cros_ec_lpc.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 7677ab3c0ead9..4158bdeee197b 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -530,23 +530,31 @@ static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
#ifdef CONFIG_PM_SLEEP
-static int cros_ec_lpc_suspend(struct device *dev)
+static int cros_ec_lpc_prepare(struct device *dev)
{
struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
+ dev_info(dev, "Prepare EC suspend\n");
+
return cros_ec_suspend(ec_dev);
}
-static int cros_ec_lpc_resume(struct device *dev)
+static void cros_ec_lpc_complete(struct device *dev)
{
struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
+ int ret;
+
+ ret = cros_ec_resume(ec_dev);
- return cros_ec_resume(ec_dev);
+ dev_info(dev, "EC resume completed: ret = %d\n", ret);
}
#endif
static const struct dev_pm_ops cros_ec_lpc_pm_ops = {
- SET_LATE_SYSTEM_SLEEP_PM_OPS(cros_ec_lpc_suspend, cros_ec_lpc_resume)
+#ifdef CONFIG_PM_SLEEP
+ .prepare = cros_ec_lpc_prepare,
+ .complete = cros_ec_lpc_complete
+#endif
};
static struct platform_driver cros_ec_lpc_driver = {
--
2.37.1.455.g008518b4e5-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] platform/chrome: cros_ec: Send host event for prepare/complete
2022-08-02 17:40 [PATCH v3] platform/chrome: cros_ec: Send host event for prepare/complete Tim Van Patten
@ 2022-08-17 21:55 ` Raul Rangel
2022-08-18 4:08 ` Tzung-Bi Shih
1 sibling, 0 replies; 6+ messages in thread
From: Raul Rangel @ 2022-08-17 21:55 UTC (permalink / raw)
To: Tim Van Patten
Cc: Rob Barnes, Benson Leung, Guenter Roeck, chrome-platform, linux-kernel
LGTM
On Tue, Aug 2, 2022 at 11:40 AM Tim Van Patten <timvp@google.com> wrote:
>
> Update cros_ec_lpc_pm_ops to call cros_ec_lpc_prepare() during PM
> .prepare() and cros_ec_lpc_complete() during .complete(). This allows the
> EC to log entry/exit of AP's suspend/resume more accurately.
>
> Signed-off-by: Tim Van Patten <timvp@google.com>
> ---
>
> Changes in v3:
> - Update cros_ec_lpc_suspend() to cros_ec_lpc_prepare()
> - Update cros_ec_lpc_resume() to cros_ec_lpc_complete()
>
> Changes in v2:
> - Include cros_ec_resume() return value in dev_info() output.
> - Guard setting .prepare/.complete with #ifdef CONFIG_PM_SLEEP.
>
> drivers/platform/chrome/cros_ec_lpc.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 7677ab3c0ead9..4158bdeee197b 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -530,23 +530,31 @@ static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
> MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
>
> #ifdef CONFIG_PM_SLEEP
> -static int cros_ec_lpc_suspend(struct device *dev)
> +static int cros_ec_lpc_prepare(struct device *dev)
> {
> struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
>
> + dev_info(dev, "Prepare EC suspend\n");
> +
> return cros_ec_suspend(ec_dev);
> }
>
> -static int cros_ec_lpc_resume(struct device *dev)
> +static void cros_ec_lpc_complete(struct device *dev)
> {
> struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = cros_ec_resume(ec_dev);
>
> - return cros_ec_resume(ec_dev);
> + dev_info(dev, "EC resume completed: ret = %d\n", ret);
> }
> #endif
>
> static const struct dev_pm_ops cros_ec_lpc_pm_ops = {
> - SET_LATE_SYSTEM_SLEEP_PM_OPS(cros_ec_lpc_suspend, cros_ec_lpc_resume)
> +#ifdef CONFIG_PM_SLEEP
> + .prepare = cros_ec_lpc_prepare,
> + .complete = cros_ec_lpc_complete
> +#endif
> };
>
> static struct platform_driver cros_ec_lpc_driver = {
> --
> 2.37.1.455.g008518b4e5-goog
>
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] platform/chrome: cros_ec: Send host event for prepare/complete
2022-08-02 17:40 [PATCH v3] platform/chrome: cros_ec: Send host event for prepare/complete Tim Van Patten
2022-08-17 21:55 ` Raul Rangel
@ 2022-08-18 4:08 ` Tzung-Bi Shih
2022-08-22 16:21 ` Tim Van Patten
1 sibling, 1 reply; 6+ messages in thread
From: Tzung-Bi Shih @ 2022-08-18 4:08 UTC (permalink / raw)
To: Tim Van Patten
Cc: rrangel, robbarnes, Benson Leung, Guenter Roeck, chrome-platform,
linux-kernel
On Tue, Aug 02, 2022 at 11:40:08AM -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 allows the
> EC to log entry/exit of AP's suspend/resume more accurately.
As what I commented on [1], the term "host event" in the commit title is
confusing. Also, as this is a cros_ec_lpc specific patch, please change
the prefix.
[1]: https://patchwork.kernel.org/project/chrome-platform/patch/20220706205136.v2.1.Ic7a7c81f880ab31533652e0928aa6e687bb268b5@changeid/#24934911
> -static int cros_ec_lpc_resume(struct device *dev)
> +static void cros_ec_lpc_complete(struct device *dev)
> {
> struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = cros_ec_resume(ec_dev);
>
> - return cros_ec_resume(ec_dev);
> + dev_info(dev, "EC resume completed: ret = %d\n", ret);
cros_ec_resume() always returns 0.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] platform/chrome: cros_ec: Send host event for prepare/complete
2022-08-18 4:08 ` Tzung-Bi Shih
@ 2022-08-22 16:21 ` Tim Van Patten
2022-08-23 2:49 ` Tzung-Bi Shih
0 siblings, 1 reply; 6+ messages in thread
From: Tim Van Patten @ 2022-08-22 16:21 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: rrangel, robbarnes, Benson Leung, Guenter Roeck, chrome-platform,
linux-kernel
On Wed, Aug 17, 2022 at 10:08 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Tue, Aug 02, 2022 at 11:40:08AM -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 allows the
> > EC to log entry/exit of AP's suspend/resume more accurately.
>
> As what I commented on [1], the term "host event" in the commit title is
> confusing. Also, as this is a cros_ec_lpc specific patch, please change
> the prefix.
I've updated the prefix to "cros_ec_lpc" and the title/description to
indicate that this CL moves when the host event is sent to
.prepare()/.complete().
> [1]: https://patchwork.kernel.org/project/chrome-platform/patch/20220706205136.v2.1.Ic7a7c81f880ab31533652e0928aa6e687bb268b5@changeid/#24934911
>
> > -static int cros_ec_lpc_resume(struct device *dev)
> > +static void cros_ec_lpc_complete(struct device *dev)
> > {
> > struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + ret = cros_ec_resume(ec_dev);
> >
> > - return cros_ec_resume(ec_dev);
> > + dev_info(dev, "EC resume completed: ret = %d\n", ret);
>
> cros_ec_resume() always returns 0.
Yes, it always returns 0 today, but that may not be the case forever.
While "ret" is not returned by cros_ec_resume() today, it's possible
for it to be non-zero and someone may update cros_ec_resume() to
return that status.
https://github.com/torvalds/linux/blob/1c23f9e627a7b412978b4e852793c5e3c3efc555/drivers/platform/chrome/cros_ec.c#L378
Additionally, everyone passes along the return code of cros_ec_resume():
drivers/platform/chrome/cros_ec_i2c.c
331: return cros_ec_resume(ec_dev);
drivers/platform/chrome/cros_ec_rpmsg.c
285: return cros_ec_resume(ec_dev);
drivers/platform/chrome/cros_ec_ishtp.c
766: return cros_ec_resume(client_data->ec_dev);
drivers/platform/chrome/cros_ec_spi.c
808: return cros_ec_resume(ec_dev);
drivers/platform/chrome/cros_ec_uart.c
390: return cros_ec_resume(ec_dev);
drivers/platform/chrome/cros_ec_lpc.c
530: return cros_ec_resume(ec_dev);
I'm going to leave this as-is, since it's good practice to check
return values (and log bad status) and so we don't need to update this
code in case cros_ec_resume() does return a non-zero value in the
future.
--
Tim Van Patten | ChromeOS | timvp@google.com | (720) 432-0997
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] platform/chrome: cros_ec: Send host event for prepare/complete
2022-08-22 16:21 ` Tim Van Patten
@ 2022-08-23 2:49 ` Tzung-Bi Shih
2022-08-23 16:00 ` Tim Van Patten
0 siblings, 1 reply; 6+ messages in thread
From: Tzung-Bi Shih @ 2022-08-23 2:49 UTC (permalink / raw)
To: Tim Van Patten
Cc: rrangel, robbarnes, Benson Leung, Guenter Roeck, chrome-platform,
linux-kernel
On Mon, Aug 22, 2022 at 10:21:47AM -0600, Tim Van Patten wrote:
> On Wed, Aug 17, 2022 at 10:08 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > On Tue, Aug 02, 2022 at 11:40:08AM -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 allows the
> > > EC to log entry/exit of AP's suspend/resume more accurately.
> >
> > As what I commented on [1], the term "host event" in the commit title is
> > confusing. Also, as this is a cros_ec_lpc specific patch, please change
> > the prefix.
>
> I've updated the prefix to "cros_ec_lpc" and the title/description to
> indicate that this CL moves when the host event is sent to
> .prepare()/.complete().
>
> > [1]: https://patchwork.kernel.org/project/chrome-platform/patch/20220706205136.v2.1.Ic7a7c81f880ab31533652e0928aa6e687bb268b5@changeid/#24934911
I'm not sure if any unclear. "host event" is a terminology for CrOS EC.
The usage here is confusing.
> >
> > > -static int cros_ec_lpc_resume(struct device *dev)
> > > +static void cros_ec_lpc_complete(struct device *dev)
> > > {
> > > struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
> > > + int ret;
> > > +
> > > + ret = cros_ec_resume(ec_dev);
> > >
> > > - return cros_ec_resume(ec_dev);
> > > + dev_info(dev, "EC resume completed: ret = %d\n", ret);
> >
> > cros_ec_resume() always returns 0.
>
> Yes, it always returns 0 today, but that may not be the case forever.
> While "ret" is not returned by cros_ec_resume() today, it's possible
> for it to be non-zero and someone may update cros_ec_resume() to
> return that status.
Does it really need to print if `ret` is always 0?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] platform/chrome: cros_ec: Send host event for prepare/complete
2022-08-23 2:49 ` Tzung-Bi Shih
@ 2022-08-23 16:00 ` Tim Van Patten
0 siblings, 0 replies; 6+ messages in thread
From: Tim Van Patten @ 2022-08-23 16:00 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: rrangel, robbarnes, Benson Leung, Guenter Roeck, chrome-platform,
linux-kernel
On Mon, Aug 22, 2022 at 8:49 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Mon, Aug 22, 2022 at 10:21:47AM -0600, Tim Van Patten wrote:
> > On Wed, Aug 17, 2022 at 10:08 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > >
> > > On Tue, Aug 02, 2022 at 11:40:08AM -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 allows the
> > > > EC to log entry/exit of AP's suspend/resume more accurately.
> > >
> > > As what I commented on [1], the term "host event" in the commit title is
> > > confusing. Also, as this is a cros_ec_lpc specific patch, please change
> > > the prefix.
> >
> > I've updated the prefix to "cros_ec_lpc" and the title/description to
> > indicate that this CL moves when the host event is sent to
> > .prepare()/.complete().
> >
> > > [1]: https://patchwork.kernel.org/project/chrome-platform/patch/20220706205136.v2.1.Ic7a7c81f880ab31533652e0928aa6e687bb268b5@changeid/#24934911
>
> I'm not sure if any unclear. "host event" is a terminology for CrOS EC.
> The usage here is confusing.
I updated the title/description to "host command".
>
> > >
> > > > -static int cros_ec_lpc_resume(struct device *dev)
> > > > +static void cros_ec_lpc_complete(struct device *dev)
> > > > {
> > > > struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
> > > > + int ret;
> > > > +
> > > > + ret = cros_ec_resume(ec_dev);
> > > >
> > > > - return cros_ec_resume(ec_dev);
> > > > + dev_info(dev, "EC resume completed: ret = %d\n", ret);
> > >
> > > cros_ec_resume() always returns 0.
> >
> > Yes, it always returns 0 today, but that may not be the case forever.
> > While "ret" is not returned by cros_ec_resume() today, it's possible
> > for it to be non-zero and someone may update cros_ec_resume() to
> > return that status.
>
> Does it really need to print if `ret` is always 0?
Yes, we should always print the return value in case it changes in the future.
--
Tim Van Patten | ChromeOS | timvp@google.com | (720) 432-0997
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-23 16:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-02 17:40 [PATCH v3] platform/chrome: cros_ec: Send host event for prepare/complete Tim Van Patten
2022-08-17 21:55 ` Raul Rangel
2022-08-18 4:08 ` Tzung-Bi Shih
2022-08-22 16:21 ` Tim Van Patten
2022-08-23 2:49 ` Tzung-Bi Shih
2022-08-23 16:00 ` Tim Van Patten
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.