diff mbox series

platform/chrome: cros_ec: Send host event for prepare/complete

Message ID 20220701095421.1.I78ded92e416b55de31975686d34b2058d4761c07@changeid (mailing list archive)
State Superseded
Headers show
Series platform/chrome: cros_ec: Send host event for prepare/complete | expand

Commit Message

Tim Van Patten July 1, 2022, 3:54 p.m. UTC
Update cros_ec_lpc_pm_ops to call cros_ec_lpc_suspend() during PM
.prepare() and cros_ec_lpc_resume() 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>
---

 drivers/platform/chrome/cros_ec_lpc.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Guenter Roeck July 1, 2022, 4:58 p.m. UTC | #1
On Fri, Jul 1, 2022 at 8:54 AM Tim Van Patten <timvp@google.com> wrote:
>
> Update cros_ec_lpc_pm_ops to call cros_ec_lpc_suspend() during PM
> .prepare() and cros_ec_lpc_resume() 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>
> ---
>
>  drivers/platform/chrome/cros_ec_lpc.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 7677ab3c0ead9..783a0e56bf5f3 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -534,19 +534,24 @@ static int cros_ec_lpc_suspend(struct device *dev)
>  {
>         struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
>
> +       dev_info(dev, "Prepare EC suspend\n");

I don't see why that logging noise is necessary and/or adds value. If
every driver did that, the entire kernel log would be polluted by
similar messages.

> +
>         return cros_ec_suspend(ec_dev);
>  }
>
> -static int cros_ec_lpc_resume(struct device *dev)
> +static void cros_ec_lpc_resume(struct device *dev)
>  {
>         struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
>
> -       return cros_ec_resume(ec_dev);
> +       cros_ec_resume(ec_dev);
> +
> +       dev_info(dev, "EC resume completed\n");

Same here.

Guenter

>  }
>  #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)
> +       .prepare = cros_ec_lpc_suspend,
> +       .complete = cros_ec_lpc_resume
>  };
>
>  static struct platform_driver cros_ec_lpc_driver = {
> --
> 2.37.0.rc0.161.g10f37bed90-goog
>
kernel test robot July 1, 2022, 8:39 p.m. UTC | #2
Hi Tim,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on chrome-platform/for-next]
[also build test ERROR on linus/master v5.19-rc4 next-20220701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Tim-Van-Patten/platform-chrome-cros_ec-Send-host-event-for-prepare-complete/20220701-235602
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
config: i386-randconfig-a001 (https://download.01.org/0day-ci/archive/20220702/202207020433.aSSwzWJn-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/46055ab1171506ae76daf77f7b880087c8a9119f
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Tim-Van-Patten/platform-chrome-cros_ec-Send-host-event-for-prepare-complete/20220701-235602
        git checkout 46055ab1171506ae76daf77f7b880087c8a9119f
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/platform/chrome/cros_ec_lpc.c:553:20: error: 'cros_ec_lpc_suspend' undeclared here (not in a function); did you mean 'cros_ec_suspend'?
     553 |         .prepare = cros_ec_lpc_suspend,
         |                    ^~~~~~~~~~~~~~~~~~~
         |                    cros_ec_suspend
>> drivers/platform/chrome/cros_ec_lpc.c:554:21: error: 'cros_ec_lpc_resume' undeclared here (not in a function); did you mean 'cros_ec_lpc_remove'?
     554 |         .complete = cros_ec_lpc_resume
         |                     ^~~~~~~~~~~~~~~~~~
         |                     cros_ec_lpc_remove


vim +553 drivers/platform/chrome/cros_ec_lpc.c

   551	
   552	static const struct dev_pm_ops cros_ec_lpc_pm_ops = {
 > 553		.prepare = cros_ec_lpc_suspend,
 > 554		.complete = cros_ec_lpc_resume
   555	};
   556
Tim Van Patten July 14, 2022, 6:17 p.m. UTC | #3
[Resending in plain text mode.]

Hi Guenter,

The PM subsystem doesn't print out the prepare/complete callbacks, so
we've added them to keep parity with the rest of the system that has
this output.


On Fri, Jul 1, 2022 at 3:16 PM Tim Van Patten <timvp@google.com> wrote:
>
> Hi Guenter,
>
> The PM subsystem doesn't print out the prepare/complete callbacks, so we've added them to keep parity with the rest of the system that has this output.
>
> On Fri, Jul 1, 2022 at 2:40 PM kernel test robot <lkp@intel.com> wrote:
>>
>> Hi Tim,
>>
>> Thank you for the patch! Yet something to improve:
>>
>> [auto build test ERROR on chrome-platform/for-next]
>> [also build test ERROR on linus/master v5.19-rc4 next-20220701]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch]
>>
>> url:    https://github.com/intel-lab-lkp/linux/commits/Tim-Van-Patten/platform-chrome-cros_ec-Send-host-event-for-prepare-complete/20220701-235602
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
>> config: i386-randconfig-a001 (https://download.01.org/0day-ci/archive/20220702/202207020433.aSSwzWJn-lkp@intel.com/config)
>> compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
>> reproduce (this is a W=1 build):
>>         # https://github.com/intel-lab-lkp/linux/commit/46055ab1171506ae76daf77f7b880087c8a9119f
>>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>>         git fetch --no-tags linux-review Tim-Van-Patten/platform-chrome-cros_ec-Send-host-event-for-prepare-complete/20220701-235602
>>         git checkout 46055ab1171506ae76daf77f7b880087c8a9119f
>>         # save the config file
>>         mkdir build_dir && cp config build_dir/.config
>>         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
>>
>> If you fix the issue, kindly add following tag where applicable
>> Reported-by: kernel test robot <lkp@intel.com>
>>
>> All errors (new ones prefixed by >>):
>>
>> >> drivers/platform/chrome/cros_ec_lpc.c:553:20: error: 'cros_ec_lpc_suspend' undeclared here (not in a function); did you mean 'cros_ec_suspend'?
>>      553 |         .prepare = cros_ec_lpc_suspend,
>>          |                    ^~~~~~~~~~~~~~~~~~~
>>          |                    cros_ec_suspend
>> >> drivers/platform/chrome/cros_ec_lpc.c:554:21: error: 'cros_ec_lpc_resume' undeclared here (not in a function); did you mean 'cros_ec_lpc_remove'?
>>      554 |         .complete = cros_ec_lpc_resume
>>          |                     ^~~~~~~~~~~~~~~~~~
>>          |                     cros_ec_lpc_remove
>>
>>
>> vim +553 drivers/platform/chrome/cros_ec_lpc.c
>>
>>    551
>>    552  static const struct dev_pm_ops cros_ec_lpc_pm_ops = {
>>  > 553          .prepare = cros_ec_lpc_suspend,
>>  > 554          .complete = cros_ec_lpc_resume
>>    555  };
>>    556
>>
>> --
>> 0-DAY CI Kernel Test Service
>> https://01.org/lkp
>
>
>
> --
>
> Tim Van Patten | ChromeOS | timvp@google.com | (720) 432-0997
diff mbox series

Patch

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 7677ab3c0ead9..783a0e56bf5f3 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -534,19 +534,24 @@  static int cros_ec_lpc_suspend(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_resume(struct device *dev)
 {
 	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
 
-	return cros_ec_resume(ec_dev);
+	cros_ec_resume(ec_dev);
+
+	dev_info(dev, "EC resume completed\n");
 }
 #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)
+	.prepare = cros_ec_lpc_suspend,
+	.complete = cros_ec_lpc_resume
 };
 
 static struct platform_driver cros_ec_lpc_driver = {