| 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
| ||
|
Message-ID: <CACwGwOEbQZCQNK6=2dOS-i54Xme34biV5j2hacDW1AKYt9gRiw@mail.gmail.com>
Date: Tue, 17 Oct 2023 10:50:54 -0500
From: Lalithkumar Rajendran <lalithkraj@...omium.org>
To: LKML <linux-kernel@...r.kernel.org>
Cc: Benson Leung <bleung@...omium.org>,
Enric Balletbo i Serra <enric.balletbo@...labora.com>,
Guenter Roeck <groeck@...omium.org>,
chrome-platform@...ts.linux.dev
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@...omium.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@...omium.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
>
Powered by blists - more mailing lists