All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Emre Cecanpunar <emreleno@gmail.com>
Cc: platform-driver-x86@vger.kernel.org,
	Hans de Goede <hansg@kernel.org>,
	 LKML <linux-kernel@vger.kernel.org>,
	krishna.chomal108@gmail.com
Subject: Re: [PATCH v2 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer
Date: Mon, 23 Mar 2026 11:51:23 +0200 (EET)	[thread overview]
Message-ID: <eae62ec1-ba14-6717-3c17-e6838feccda9@linux.intel.com> (raw)
In-Reply-To: <20260322190624.35162-4-emreleno@gmail.com>

On Sun, 22 Mar 2026, Emre Cecanpunar wrote:

> The keep-alive mechanism re-applies fan settings every 90 seconds to
> prevent the firmware from reverting to automatic mode after its 120 s
> timeout. Fan settings are also applied immediately when the user writes
> to the hwmon sysfs interface.
> 
> schedule_delayed_work() is a no-op when the work item is already
> pending. If the user updates fan settings at T=85s, the keep-alive
> still fires at T=90s (5 s after the update) instead of 90 s later.
> This shortens the effective keep-alive window and may allow the
> firmware timeout to expire between cycles.

Hi,

Unfortunately, I don't understand this explanation.

Won't the keepalive expiry at T=90s just result in adding the next 
keepaline event to T=90s + 90s, and the resulting intervals never exceeds 
120s?

It may be inefficient to apply fan settings at T=85s and then fire 
keepalive at T=90s, that much I can see but if you think there's a problem 
beyond that, this explanation doesn't convince me so please elaborate.

If the only problem is inefficiency, this explanation did not convey 
that meaning so in that case, please rephrase.

> Replace schedule_delayed_work() with mod_delayed_work() in both the
> PWM_MODE_MAX and PWM_MODE_MANUAL cases. mod_delayed_work() resets the
> delay to KEEP_ALIVE_DELAY_SECS from the time of each call, whether or
> not the work is already pending, ensuring each user action and each
> keep-alive expiry restarts the full 90 s window.
> 
> Signed-off-by: Emre Cecanpunar <emreleno@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index 41769c0861e5..a29f34588055 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -2342,8 +2342,8 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
>  		ret = hp_wmi_fan_speed_max_set(1);
>  		if (ret < 0)
>  			return ret;
> -		schedule_delayed_work(&priv->keep_alive_dwork,
> -				      secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
> +		mod_delayed_work(system_wq, &priv->keep_alive_dwork,
> +				 secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
>  		return 0;
>  	case PWM_MODE_MANUAL:
>  		if (!is_victus_s_thermal_profile())
> @@ -2351,8 +2351,8 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
>  		ret = hp_wmi_fan_speed_set(priv, pwm_to_rpm(priv->pwm, priv));
>  		if (ret < 0)
>  			return ret;
> -		schedule_delayed_work(&priv->keep_alive_dwork,
> -				      secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
> +		mod_delayed_work(system_wq, &priv->keep_alive_dwork,
> +				 secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
>  		return 0;
>  	case PWM_MODE_AUTO:
>  		if (is_victus_s_thermal_profile()) {
> 

-- 
 i.


  reply	other threads:[~2026-03-23  9:51 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-22 19:06 [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes Emre Cecanpunar
2026-03-22 19:06 ` [PATCH v2 1/5] platform/x86: hp-wmi: fix ignored return values in fan settings Emre Cecanpunar
2026-03-22 19:06 ` [PATCH v2 2/5] platform/x86: hp-wmi: avoid cancel_delayed_work_sync from work handler Emre Cecanpunar
2026-03-22 19:06 ` [PATCH v2 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer Emre Cecanpunar
2026-03-23  9:51   ` Ilpo Järvinen [this message]
2026-03-22 19:06 ` [PATCH v2 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
2026-03-23  9:42   ` Ilpo Järvinen
2026-03-22 19:06 ` [PATCH v2 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access Emre Cecanpunar
2026-03-24 15:46 ` [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes Ilpo Järvinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eae62ec1-ba14-6717-3c17-e6838feccda9@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=emreleno@gmail.com \
    --cc=hansg@kernel.org \
    --cc=krishna.chomal108@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.