linux-kernel - Re: [PATCH v5 1/2] clocksource: acpi_pm: Add external callback for suspend/resume

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <935e8c82-3c91-4c9a-8e43-e6045b28279d@redhat.com>
Date: Mon, 12 Aug 2024 10:03:39 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Marek Maslanka <mmaslanka@...gle.com>, LKML <linux-kernel@...r.kernel.org>
Cc: Daniel Lezcano <daniel.lezcano@...aro.org>,
 Thomas Gleixner <tglx@...utronix.de>,
 Rajneesh Bhardwaj <irenic.rajneesh@...il.com>,
 David E Box <david.e.box@...el.com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v5 1/2] clocksource: acpi_pm: Add external callback for
 suspend/resume

Hi,

Thank you for your patch.

On 8/12/24 6:37 AM, Marek Maslanka wrote:
> Provides the capability to register an external callback for the ACPI PM
> timer, which is called during the suspend and resume processes.
> 
> Signed-off-by: Marek Maslanka <mmaslanka@...gle.com>
> 
> ---
> Changes in v5:
> - Rename acpi_pm_register_suspend_resume_callback to
>   acpi_pmtmr_register_suspend_resume_callback and move prototype to
>   include/linux/acpi_pmtmr.h
> - Remove the acpi_pm.h header added in the previous patch.
> - Link to v4: https://lore.kernel.org/lkml/20240809131343.1173369-1-mmaslanka@google.com/
> ---
> ---
>  drivers/clocksource/acpi_pm.c | 24 ++++++++++++++++++++++++
>  include/linux/acpi_pmtmr.h    |  8 ++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c
> index 82338773602ca..fab19b7de55c1 100644
> --- a/drivers/clocksource/acpi_pm.c
> +++ b/drivers/clocksource/acpi_pm.c
> @@ -25,6 +25,10 @@
>  #include <asm/io.h>
>  #include <asm/time.h>
>  
> +static void *suspend_resume_cb_data;
> +
> +static void (*suspend_resume_callback)(void *data, bool suspend);
> +
>  /*
>   * The I/O port the PMTMR resides at.
>   * The location is detected during setup_arch(),
> @@ -58,6 +62,24 @@ u32 acpi_pm_read_verified(void)
>  	return v2;
>  }
>  
> +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data)
> +{
> +	suspend_resume_callback = cb;
> +	suspend_resume_cb_data = data;
> +}

The intel-pmc driver which is a consumer of this symbol can be build as
a module, so this needs a EXPORT_SYMBOL_GPL().

Also the pmc driver can be unbound from its device, or the entire
module can be removed, so this also needs an unregister function
to match, so that the pmc driver can unregister its callback
from pmc_core_remove().

Regards,

Hans



> +
> +static void acpi_pm_suspend(struct clocksource *cs)
> +{
> +	if (suspend_resume_callback)
> +		suspend_resume_callback(suspend_resume_cb_data, true);
> +}
> +
> +static void acpi_pm_resume(struct clocksource *cs)
> +{
> +	if (suspend_resume_callback)
> +		suspend_resume_callback(suspend_resume_cb_data, false);
> +}
> +
>  static u64 acpi_pm_read(struct clocksource *cs)
>  {
>  	return (u64)read_pmtmr();
> @@ -69,6 +91,8 @@ static struct clocksource clocksource_acpi_pm = {
>  	.read		= acpi_pm_read,
>  	.mask		= (u64)ACPI_PM_MASK,
>  	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
> +	.suspend	= acpi_pm_suspend,
> +	.resume		= acpi_pm_resume,
>  };
>  
>  
> diff --git a/include/linux/acpi_pmtmr.h b/include/linux/acpi_pmtmr.h
> index 50d88bf1498d7..a5262d28b97e0 100644
> --- a/include/linux/acpi_pmtmr.h
> +++ b/include/linux/acpi_pmtmr.h
> @@ -26,6 +26,14 @@ static inline u32 acpi_pm_read_early(void)
>  	return acpi_pm_read_verified() & ACPI_PM_MASK;
>  }
>  
> +/**
> + * Register callback for suspend and resume event
> + *
> + * @cb Callback triggered on suspend and resume
> + * @data Data passed with the callback
> + */
> +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data);
> +
>  #else
>  
>  static inline u32 acpi_pm_read_early(void)