Messages in this thread | | | From | Lalithkumar Rajendran <> | Date | Tue, 17 Oct 2023 10:50:54 -0500 | Subject | Re: [PATCH] platform/chrome: cros_ec_lpc: Separate host command and irq disable |
| |
Adding chrome-platform to CC.
On Tue, Oct 10, 2023 at 12:03 PM Lalith Rajendran <lalithkraj@chromium.org> wrote: > > Both cros host command and irq disable were moved to suspend > prepare stage from late suspend recently. This is causing EC > to report MKBP event timeouts during suspend stress testing. > When the MKBP event timeouts happen during suspend, subsequent > wakeup of AP by EC using MKBP doesn't happen properly. Although > there are other issues to debug here, this change move the irq > disabling part back to late suspend stage which is a general > suggestion from the suspend kernel documentaiton to do irq > disable as late as possible. > > Signed-off-by: Lalith Rajendran <lalithkraj@chromium.org> > --- > > drivers/platform/chrome/cros_ec.c | 90 ++++++++++++++++++++++++--- > drivers/platform/chrome/cros_ec.h | 4 ++ > drivers/platform/chrome/cros_ec_lpc.c | 21 +++++-- > 3 files changed, 101 insertions(+), 14 deletions(-) > > diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c > index 2b49155a9b35..6f520c13c0f3 100644 > --- a/drivers/platform/chrome/cros_ec.c > +++ b/drivers/platform/chrome/cros_ec.c > @@ -317,16 +317,17 @@ EXPORT_SYMBOL(cros_ec_unregister); > > #ifdef CONFIG_PM_SLEEP > /** > - * cros_ec_suspend() - Handle a suspend operation for the ChromeOS EC device. > + * cros_ec_suspend_prepare() - Handle a suspend prepare operation for the ChromeOS EC device. > * @ec_dev: Device to suspend. > * > - * This can be called by drivers to handle a suspend event. > + * This can be called by drivers to handle a suspend prepare stage of suspend. > + * Drivers should either call cros_ec_supsend or call > + * cros_ec_suspend_prepare and cros_ec_suspend_late. > * > * Return: 0 on success or negative error code. > */ > -int cros_ec_suspend(struct cros_ec_device *ec_dev) > +int cros_ec_suspend_prepare(struct cros_ec_device *ec_dev) > { > - struct device *dev = ec_dev->dev; > int ret; > u8 sleep_event; > > @@ -338,7 +339,23 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev) > if (ret < 0) > dev_dbg(ec_dev->dev, "Error %d sending suspend event to ec", > ret); > + return 0; > +} > +EXPORT_SYMBOL(cros_ec_suspend_prepare); > > +/** > + * cros_ec_suspend_late() - Handle a suspend late operation for the ChromeOS EC device. > + * @ec_dev: Device to suspend. > + * > + * This can be called by drivers to handle a suspend late stage of suspend. > + * Drivers should either call cros_ec_supsend or call > + * cros_ec_suspend_prepare and cros_ec_suspend_late. > + * > + * Return: 0 on success or negative error code. > + */ > +int cros_ec_suspend_late(struct cros_ec_device *ec_dev) > +{ > + struct device *dev = ec_dev->dev; > if (device_may_wakeup(dev)) > ec_dev->wake_enabled = !enable_irq_wake(ec_dev->irq); > > @@ -348,6 +365,24 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev) > > return 0; > } > +EXPORT_SYMBOL(cros_ec_suspend_late); > + > +/** > + * cros_ec_suspend() - Handle a suspend operation for the ChromeOS EC device. > + * @ec_dev: Device to suspend. > + * > + * This can be called by drivers to handle a suspend event. > + * Drivers should either call cros_ec_supsend or call > + * cros_ec_suspend_prepare and cros_ec_suspend_late. > + * > + * Return: 0 on success or negative error code. > + */ > +int cros_ec_suspend(struct cros_ec_device *ec_dev) > +{ > + cros_ec_suspend_prepare(ec_dev); > + cros_ec_suspend_late(ec_dev); > + return 0; > +} > EXPORT_SYMBOL(cros_ec_suspend); > > static void cros_ec_report_events_during_suspend(struct cros_ec_device *ec_dev) > @@ -365,21 +400,20 @@ static void cros_ec_report_events_during_suspend(struct cros_ec_device *ec_dev) > } > > /** > - * cros_ec_resume() - Handle a resume operation for the ChromeOS EC device. > + * cros_ec_resume() - Handle a resume complete operation for the ChromeOS EC device. > * @ec_dev: Device to resume. > * > - * This can be called by drivers to handle a resume event. > + * This can be called by drivers to handle a resume complete stage of resume. > + * Drivers should either call cros_ec_resume or call > + * cros_ec_resume_early and cros_ec_resume_complete. > * > * Return: 0 on success or negative error code. > */ > -int cros_ec_resume(struct cros_ec_device *ec_dev) > +int cros_ec_resume_complete(struct cros_ec_device *ec_dev) > { > int ret; > u8 sleep_event; > > - ec_dev->suspended = false; > - enable_irq(ec_dev->irq); > - > sleep_event = (!IS_ENABLED(CONFIG_ACPI) || pm_suspend_via_firmware()) ? > HOST_SLEEP_EVENT_S3_RESUME : > HOST_SLEEP_EVENT_S0IX_RESUME; > @@ -388,6 +422,24 @@ int cros_ec_resume(struct cros_ec_device *ec_dev) > if (ret < 0) > dev_dbg(ec_dev->dev, "Error %d sending resume event to ec", > ret); > + return 0; > +} > +EXPORT_SYMBOL(cros_ec_resume_complete); > + > +/** > + * cros_ec_resume_early() - Handle a resume early operation for the ChromeOS EC device. > + * @ec_dev: Device to resume. > + * > + * This can be called by drivers to handle a resume early stage of resume. > + * Drivers should either call cros_ec_resume or call > + * cros_ec_resume_early and cros_ec_resume_complete. > + * > + * Return: 0 on success or negative error code. > + */ > +int cros_ec_resume_early(struct cros_ec_device *ec_dev) > +{ > + ec_dev->suspended = false; > + enable_irq(ec_dev->irq); > > if (ec_dev->wake_enabled) { > disable_irq_wake(ec_dev->irq); > @@ -402,6 +454,24 @@ int cros_ec_resume(struct cros_ec_device *ec_dev) > > return 0; > } > +EXPORT_SYMBOL(cros_ec_resume_early); > + > +/** > + * cros_ec_resume() - Handle a resume operation for the ChromeOS EC device. > + * @ec_dev: Device to resume. > + * > + * This can be called by drivers to handle a resume event. > + * Drivers should either call cros_ec_resume or call > + * cros_ec_resume_early and cros_ec_resume_complete. > + * > + * Return: 0 on success or negative error code. > + */ > +int cros_ec_resume(struct cros_ec_device *ec_dev) > +{ > + cros_ec_resume_early(ec_dev); > + cros_ec_resume_complete(ec_dev); > + return 0; > +} > EXPORT_SYMBOL(cros_ec_resume); > > #endif > diff --git a/drivers/platform/chrome/cros_ec.h b/drivers/platform/chrome/cros_ec.h > index bbca0096868a..41defaa5e766 100644 > --- a/drivers/platform/chrome/cros_ec.h > +++ b/drivers/platform/chrome/cros_ec.h > @@ -14,7 +14,11 @@ int cros_ec_register(struct cros_ec_device *ec_dev); > void cros_ec_unregister(struct cros_ec_device *ec_dev); > > int cros_ec_suspend(struct cros_ec_device *ec_dev); > +int cros_ec_suspend_late(struct cros_ec_device *ec_dev); > +int cros_ec_suspend_prepare(struct cros_ec_device *ec_dev); > int cros_ec_resume(struct cros_ec_device *ec_dev); > +int cros_ec_resume_early(struct cros_ec_device *ec_dev); > +int cros_ec_resume_complete(struct cros_ec_device *ec_dev); > > irqreturn_t cros_ec_irq_thread(int irq, void *data); > > diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c > index 8982cf23e514..afb9f7dbb2ba 100644 > --- a/drivers/platform/chrome/cros_ec_lpc.c > +++ b/drivers/platform/chrome/cros_ec_lpc.c > @@ -537,22 +537,35 @@ MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table); > static int cros_ec_lpc_prepare(struct device *dev) > { > struct cros_ec_device *ec_dev = dev_get_drvdata(dev); > - > - return cros_ec_suspend(ec_dev); > + return cros_ec_suspend_prepare(ec_dev); > } > > static void cros_ec_lpc_complete(struct device *dev) > { > struct cros_ec_device *ec_dev = dev_get_drvdata(dev); > - cros_ec_resume(ec_dev); > + cros_ec_resume_complete(ec_dev); > +} > +static int cros_ec_lpc_suspend_late(struct device *dev) > +{ > + struct cros_ec_device *ec_dev = dev_get_drvdata(dev); > + > + return cros_ec_suspend_late(ec_dev); > +} > + > +static int cros_ec_lpc_resume_early(struct device *dev) > +{ > + struct cros_ec_device *ec_dev = dev_get_drvdata(dev); > + > + return cros_ec_resume_early(ec_dev); > } > #endif > > static const struct dev_pm_ops cros_ec_lpc_pm_ops = { > #ifdef CONFIG_PM_SLEEP > .prepare = cros_ec_lpc_prepare, > - .complete = cros_ec_lpc_complete > + .complete = cros_ec_lpc_complete, > #endif > + SET_LATE_SYSTEM_SLEEP_PM_OPS(cros_ec_lpc_suspend_late, cros_ec_lpc_resume_early) > }; > > static struct platform_driver cros_ec_lpc_driver = { > -- > 2.42.0.609.gbb76f46606-goog >
| |