From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f51.google.com (mail-pj1-f51.google.com [209.85.216.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 04FCA23F417 for ; Tue, 30 Dec 2025 14:33:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767105198; cv=none; b=aWvoprP2e2dLsHuVRt07omv1CkBP8zG8xTdAsJxSE1gqUg05hwoU1RGiaGySYGRkV1TnfocDpsCOPEmZG7HTIqGYlALjqpvfOEDM+j3bdYh9ZGYR5w4EUCYI3MIUP6Ile1yAH3TSV/VWkanvkrDxWH3xxWuxtbqA4rbNM7R8Lzw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767105198; c=relaxed/simple; bh=KunfU9CiyU08bAvs3zwe8a8Sz4+i9wJ9jB8yL0XNj0c=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=XPo6GtBwWeFk/LQ8SHPiYF90NuNIge4gE+S0Watx3rd5JXEmj16mLyXkVmXm8LrwXlrsFis3XJUu3Q7uHAcU1pRM8VL/DpzAidS1LiHsaehIcLXL94anIeUl8T3LwJGN2CMXSQUvQULr/6uzIXaqKvKDpaEP3lrT0m5KcfUn7mo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=P4477bNF; arc=none smtp.client-ip=209.85.216.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="P4477bNF" Received: by mail-pj1-f51.google.com with SMTP id 98e67ed59e1d1-34abc7da414so8070849a91.0 for ; Tue, 30 Dec 2025 06:33:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1767105196; x=1767709996; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=jeORyyYDYkSbyJvNmNvW6nrc/284HDtM+eQ3+Nxnq8U=; b=P4477bNFwdSM2185w/OAL08hSltiQfklIKNFXyykam5U2gXq/UPO/StT8QnprSgsJ1 YM6r81FzuVjw9eQ/fA0ttPVusimMAMJmIFgKgpp4ldiFRTSRNcULCXDuj6VjHJGm3NOf RTXu60uN4JxGSxvt0maEwI+Gwx6C1tCdTlP7Usr+Du6RJ8LkOTvBIyuIoxmsTt++n+BD i17k6RY+sgo9hkm5tAUEecyn1C8fZfVRV6yDn9Wlfq9l9XDLC1yYDOeLkljJGg9Sl1ad MQcQZDU72l7sp2iywzU0s7ZHy6TdmnpvUJnP3OzUt4kIjYvpZgrelWyBoobvuU6M+Oin DCFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1767105196; x=1767709996; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=jeORyyYDYkSbyJvNmNvW6nrc/284HDtM+eQ3+Nxnq8U=; b=oLadJpMhTKkgr7/zbD+TPA4BWhCuPF+2Kl/CfhF+OPnQ3XcbGy5AXkSu6dzI9Jo+Hl 60TponDL9DKjI9vjAtxkTPpwG653Eh+boCMa8dIlravVtA7ALr99hnoE5743D+pFtg5a PRIf60Pdw1NfXEeSEI2DbeurUZnKp2ROhk00V/SRLddtY6mmyaANTi+C2mWE7A7r/gCW +fqKdAtbRvdEf7RTQM3eGZQPyGItnFa642KYb9AanxzMtg510nEHDaoslIh/sO9EEVto NUUjJxYsKv/xM78TVpNQDPAdctWm2JqCOcpNGX7oKvSaFWjk7LzpmJimd5a/WHT7+jII eTQA== X-Forwarded-Encrypted: i=1; AJvYcCVOJbX2eOtok8EC1+edREa1ecxnJC0KixpCDoujimZFLuSnx4i/a2vzE+XnT190P6pKKUt1/eodSrNgOXB8sKLXesWy@vger.kernel.org X-Gm-Message-State: AOJu0YyiKJiDyhfixpwZC2xHR3BvnL4EQE49gTf8wwZxgwh9tJdNKv56 C/BxkcQh/BK/bP1dLKvD+Cfv5JxwGgoR1a7T91CzSbgUNeHapQAbdXhu X-Gm-Gg: AY/fxX4w8oN8Uu5NpatLK2wwJykEGuBrptYwdS2BBL80TDZWJCzczWVj3DJefc+6Tik 5vzlzkr7A9xfP3kZjnU1e1AgZgu48fjxU+2KMjyfGj/h8cfuD0PRJ8jE5Df/G49hFAV5D4s1fRa JSRsaa9PTuasHG2QOjFZNJQeMEAjMUvwiFmNqVOhTTlPyMDWuHoVZ6skq4h4yvcla3GSQIsfCH7 SIm+P7yXkiWKx5ZlA6/vX4B5UlUJrEWhU6p3qNXAX5kyOG4i1VDDdXnDkV0W/bBRqE3cJ7BSsD8 j1Bk5g6GeLb5igd7r+p2RsamARaaVu+h/RVN7azoiSXatsvb5cW+jg/JLPewjZZCusdUzoC3rBU vLDE25pBo+ZKJaGpIRF43uzIOWNsLMv5TJzn/85+I1kfn15sYCH/ZVJjVHTFYMDliqYhgKeumVz CbIJo7t/avVI40aoU2Rb8= X-Google-Smtp-Source: AGHT+IHhQcFQwSL5mw4bTGmbhqwWNMktKpuulypFK5d2RP4sjL99p2VVy51z4NjoqHzyYo9oMV6m7Q== X-Received: by 2002:a17:90b:33c9:b0:32e:6fae:ba52 with SMTP id 98e67ed59e1d1-34e9211d59dmr27919455a91.6.1767105196128; Tue, 30 Dec 2025 06:33:16 -0800 (PST) Received: from archlinux ([2405:201:1b:225c:eb9d:1fc0:f95c:bd90]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-34e70d656casm33464049a91.7.2025.12.30.06.33.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 Dec 2025 06:33:15 -0800 (PST) Date: Tue, 30 Dec 2025 20:03:04 +0530 From: Krishna Chomal To: Ilpo =?utf-8?B?SsOkcnZpbmVu?= Cc: Hans de Goede , linux@roeck-us.net, platform-driver-x86@vger.kernel.org, linux-hwmon@vger.kernel.org, LKML Subject: Re: [PATCH 2/2] platform/x86: hp-wmi: implement fan keep-alive Message-ID: References: <20251225142310.204831-1-krishna.chomal108@gmail.com> <20251225142310.204831-3-krishna.chomal108@gmail.com> <7072d96f-455a-5249-2553-6e72e6a00577@linux.intel.com> Precedence: bulk X-Mailing-List: platform-driver-x86@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <7072d96f-455a-5249-2553-6e72e6a00577@linux.intel.com> On Mon, Dec 29, 2025 at 03:21:42PM +0200, Ilpo Järvinen wrote: [snip] >> +/* >> + * 90s delay to prevent the firmware from resetting fan mode after fixed >> + * 120s timeout >> + */ >> +#define KEEP_ALIVE_DELAY 90 > >For any time related define, please include its unit in the name so a >person reading code can immediately know it. > Renamed to KEEP_ALIVE_DELAY_SECS v2. [snip] >> @@ -2159,23 +2174,36 @@ static int hp_wmi_hwmon_enforce_ctx(struct hp_wmi_hwmon_priv *ctx) >> case PWM_MODE_MAX: >> if (is_victus_s_thermal_profile()) >> hp_wmi_get_fan_count_userdefine_trigger(); >> - return hp_wmi_fan_speed_max_set(1); >> + ret = hp_wmi_fan_speed_max_set(1); >> + break; >> case PWM_MODE_MANUAL: >> if (!is_victus_s_thermal_profile()) >> return -EOPNOTSUPP; >> - return hp_wmi_fan_speed_set(ctx, PWM_TO_RPM(ctx->pwm, ctx)); >> + ret = hp_wmi_fan_speed_set(ctx, PWM_TO_RPM(ctx->pwm, ctx)); >> + break; >> case PWM_MODE_AUTO: >> if (is_victus_s_thermal_profile()) { >> hp_wmi_get_fan_count_userdefine_trigger(); >> - return hp_wmi_fan_speed_max_reset(ctx); >> + ret = hp_wmi_fan_speed_max_reset(ctx); >> } else { >> - return hp_wmi_fan_speed_max_set(0); >> + ret = hp_wmi_fan_speed_max_set(0); >> } >> + break; >> default: >> /* shouldn't happen */ >> return -EINVAL; >> } >> >> + if (ret < 0) >> + return ret; >> + >> + /* Reschedule keep-alive work based on the new state */ >> + if (ctx->mode == PWM_MODE_MAX || ctx->mode == PWM_MODE_MANUAL) >> + schedule_delayed_work(&ctx->keep_alive_dwork, >> + secs_to_jiffies(KEEP_ALIVE_DELAY)); >> + else >> + cancel_delayed_work_sync(&ctx->keep_alive_dwork); > >This is now duplicating the switch/case, just add these to the existing >cases. > >You may want to introduce ret variable already in PATCH 1/2 and add a note >there that an upcoming change is going to add keep-alive so the return >value has to be stored temporarily. > Yes, handled the re-scheduling in switch/case itself in v2 and introduced the temporary ret variable in patch 1/2. >> + >> return 0; >> } >> >> @@ -2321,6 +2349,20 @@ static const struct hwmon_chip_info chip_info = { >> .info = info, >> }; >> >> +static void hp_wmi_hwmon_keep_alive_handler(struct work_struct *work) >> +{ >> + struct delayed_work *dwork; >> + struct hp_wmi_hwmon_priv *ctx; >> + >> + dwork = to_delayed_work(work); >> + ctx = container_of(dwork, struct hp_wmi_hwmon_priv, keep_alive_dwork); >> + /* >> + * Re-apply the current hwmon context settings. >> + * NOTE: hp_wmi_hwmon_enforce_ctx will handle the re-scheduling. >> + */ >> + hp_wmi_hwmon_enforce_ctx(ctx); > >(I only now understand why you named this function like this, and I still >don't think it's a good name for it as you've other callers besides this >one real "enforcing" case.) > Yes named to "apply_fan_settings" in patch 1/2 of this series.