[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240201014808.GF18560@ranerica-svr.sc.intel.com>
Date: Wed, 31 Jan 2024 17:48:08 -0800
From: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
To: Stanislaw Gruszka <stanislaw.gruszka@...ux.intel.com>
Cc: linux-pm@...r.kernel.org, "Rafael J. Wysocki" <rafael@...nel.org>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
netdev@...r.kernel.org
Subject: Re: [PATCH 3/3] thermal: intel: hfi: Enable interface only when
required
On Wed, Jan 31, 2024 at 01:05:35PM +0100, Stanislaw Gruszka wrote:
> Enable and disable hardware feedback interface (HFI) when user space
> handler is present. For example, enable HFI, when intel-speed-select or
> Intel Low Power daemon is running and subscribing to thermal netlink
> events,. When user space handlers exit or remove subscription for
> thermal netlink events, disable HFI.
I'd rephrase as:
Enable and disable HFI when a user space handler is running and listening
to thermal netlink events (e.g., intel-speed-select, Intel Low Power
daemon). When the user space handlers exit or remove subscription, disable
HFI.
>
> Summary of changes:
>
> - When CPU is online, instead of blindly enabling HFI by calling
> hfi_enable(), check if the thermal netlink group has any listener.
> This will make sure that HFI is not enabled by default during boot
> time.
s/by calling hfi_enable()//
>
> - Register a netlink notifier.
>
> - In the notifier process reason code NETLINK_CHANGE and
s/notifier/notifier,/
> NETLINK_URELEASE. If thermal netlink group has any listener enable
> HFI on all packages. If there are no listener disable HFI on all
> packages.
>
> - Actual processing to enable/disable matches what is done in
> suspend/resume callbacks. So, create two functions hfi_do_enable()
s/So,//
> and hfi_do_disable(), which can be called fromĀ the netlink notifier
> callback and suspend/resume callbacks.
>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@...ux.intel.com>
> ---
> drivers/thermal/intel/intel_hfi.c | 82 +++++++++++++++++++++++++++----
> 1 file changed, 73 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/thermal/intel/intel_hfi.c b/drivers/thermal/intel/intel_hfi.c
> index 3b04c6ec4fca..50601f75f6dc 100644
> --- a/drivers/thermal/intel/intel_hfi.c
> +++ b/drivers/thermal/intel/intel_hfi.c
> @@ -30,6 +30,7 @@
> #include <linux/kernel.h>
> #include <linux/math.h>
> #include <linux/mutex.h>
> +#include <linux/netlink.h>
> #include <linux/percpu-defs.h>
> #include <linux/printk.h>
> #include <linux/processor.h>
> @@ -480,7 +481,8 @@ void intel_hfi_online(unsigned int cpu)
> /* Enable this HFI instance if this is its first online CPU. */
> if (cpumask_weight(hfi_instance->cpus) == 1) {
> hfi_set_hw_table(hfi_instance);
> - hfi_enable();
> + if (thermal_group_has_listeners(THERMAL_GENL_EVENT_GROUP))
> + hfi_enable();
You could avoid the extra level of indentation if you did:
if (cpumask_weight() == 1 && has_listeners())
You would need to also update the comment.
My two cents.
Powered by blists - more mailing lists