From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) (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 D89BD45038 for ; Tue, 30 Dec 2025 14:33:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767105198; cv=none; b=hdktf6x7bFYEiDZE/00U3XOa7RCaSUuvuubrCyJjet/9G7+VmXvRmM2Y08mWAPakGXPG3X9ub4kTecMFUPwj4/HtUvVuFL25pbyD3oDURsHqU5ZGpUx/0+2vbfIy8xAtcJNTuq1sRFvFIobwO/QcXU4MFWCTxepQwXWuCrHYU0Y= 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.43 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-f43.google.com with SMTP id 98e67ed59e1d1-34cf1e31f85so8158613a91.1 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=q+l4y5lUNdtovayAv2T/P/RhPvq9LKMTI6cf6tGb9EL1G7KIdl9MyZKN2EoluGo8sJ Wwr6o/nVIBBzEWw+/1beWPkeL34A5TMYsfF7WQEKh9Cg/+zCNTqtOCtfceYAcXsxPCmb sPNPntC0wl+xQMZkoVtrK2HtT7Ik8eCoE6/ur1pe+hGmuKgXht/qEg3H7E1o4Q1ordBL QwlZMY7O7qvamDeHiixwPEMSF0bj3oErKef6b418iIQV+bYyrsvdst1HjiUKQTNFqO6a FsDuPinNxAX+26tYhQWRQnFFJYbQQ5/hCRtWl+VXoYG9UEQxjlrCFoKYALsLCIZU3kkE np1A== X-Forwarded-Encrypted: i=1; AJvYcCVma09TktoZ1+NI0Kh+sGtL5qN7Vxwb2yFSeilvvXIzKXtrlKF9gD1PoI4stdWEKSnwmnuYXfGoc9de1Wg=@vger.kernel.org X-Gm-Message-State: AOJu0YwwyiibgeWrHXYiyJg2fCMFD+nrFjhFg2VSvzs8ysyvScEUtIoT tA3Oag4R2rFidbcjPfTGozeGj1+MEcMw/CVX2wAag1YgKMmMxV5EFF/H X-Gm-Gg: AY/fxX7tQhqTUxjRR/9pOYlvsJhYcmhh2yhmeWmLDqLv+jxHDpOrKcBfzjjO7VJyng+ ttU4/9/TL8lRiTHj9McKiZV/gtlC5TqQyiRfmxD1Lj+ms7BzfNV19qIsAzSdfFgpaMBG4KWtNG0 aOy1uBVES3HJ9tI5RC62KwA+ab9QMr1abyV/Sh+k81CEe8V8HlwWEy5/EfYI1PqFuNrvEtZW87g zi4pBYhM6B4aoBQRmgxXs/pW9FFUHHUt30/y8OFW223JCKb4uL/OsCVNmPiz5KmbEcloDrPxxgT sxw9BCh+mkkKUatSTCT2tZC+U05UcBrnc+06Go8nxnHKQ0JQ5z368m/1NetHfBVaqgMzVYYVsHY HVpAHAtrGsdL04C2M1OmPK3JCwEMB7yF4pLqMDb748DH3GhIJVpZFw1U5vD9/LVPX80LZFDPLld Ru72bEYoc7U/Hi+xKdki4= 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: linux-kernel@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.