[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <D8O2H5QEYWPA.15LUV2KUG8MKC@gmail.com>
Date: Sun, 23 Mar 2025 21:24:03 -0300
From: "Kurt Borja" <kuurtb@...il.com>
To: "Seyediman Seyedarab" <imandevel@...il.com>, <hmh@....eng.br>,
<hdegoede@...hat.com>, <ilpo.jarvinen@...ux.intel.com>
Cc: <ibm-acpi-devel@...ts.sourceforge.net>,
<platform-driver-x86@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
"Seyediman Seyedarab" <ImanDevel@...il.com>, "Eduard Christian Dumitrescu"
<eduard.c.dumitrescu@...il.com>, "Vlastimil Holer"
<vlastimil.holer@...il.com>, "crok" <crok.bic@...il.com>, "Alireza Elikahi"
<scr0lll0ck1s4b0v3h0m3k3y@...il.com>
Subject: Re: [PATCH] platform/x86: thinkpad_acpi: disable ACPI fan access
for T495* and E560
Hi Seyediman,
On Sun Mar 23, 2025 at 12:01 AM -03, Seyediman Seyedarab wrote:
Patches submitted on behalf of someone must begin with a From: tag.
> The bug was introduced in commit 57d0557dfa49 ("platform/x86:
> thinkpad_acpi: Add Thinkpad Edge E531 fan support") which adds a new
> fan control method via the FANG and FANW ACPI methods.
>
> T945, T495s, and E560 laptops have the FANG+FANW ACPI methods (therefore
> fang_handle and fanw_handle are not NULL) but they do not actually work,
> which results in the dreaded "No such device or address" error. Fan access
> and control is restored after forcing the legacy non-ACPI fan control
> method by setting both fang_handle and fanw_handle to NULL.
>
> The DSDT table code for the FANG+FANW methods doesn't seem to do anything
> special regarding the fan being secondary.
>
> This patch adds a quirk for T495, T495s, and E560 to make them avoid the
> FANG/FANW methods.
>
>
> Original-patch-by: Eduard Christian Dumitrescu <eduard.c.dumitrescu@...il.com>
This tag is not required. Instead use Co-developed-by + Signed-off-by
tags, which are required.
> Co-authored-by: Seyediman Seyedarab <ImanDevel@...il.com>
Move this tag just before your Signed-off-by and change it to
Co-developed-by.
> Reported-by: Vlastimil Holer <vlastimil.holer@...il.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219643
This is missing a
Fixes: 57d0557dfa49 ("platform/x86: thinkpad_acpi: Add Thinkpad Edge E531 fan support")
As this is commit is already on stable, also add
Cc: stable@...r.kernel.org
> Tested-by: crok <crok.bic@...il.com>
> Tested-by: Alireza Elikahi <scr0lll0ck1s4b0v3h0m3k3y@...il.com>
> Signed-off-by: Seyediman Seyedarab <ImanDevel@...il.com>
You can go through [1] for details, before re-submitting.
> ---
> The main patch was proposed on Bugzilla, but I couldn't reach the
> original author (Eduard Christian Dumitrescu) to help him fix it
> and resend it, so I submitted it on his behalf.
>
> Kindest Regards,
> Seyediman
>
> drivers/platform/x86/thinkpad_acpi.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index d8df1405edfa..365cd7e452a4 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -8793,6 +8793,7 @@ static const struct attribute_group fan_driver_attr_group = {
> #define TPACPI_FAN_NS 0x0010 /* For EC with non-Standard register addresses */
> #define TPACPI_FAN_DECRPM 0x0020 /* For ECFW's with RPM in register as decimal */
> #define TPACPI_FAN_TPR 0x0040 /* Fan speed is in Ticks Per Revolution */
> +#define TPACPI_FAN_NOACPI 0x0080 /* Don't use ACPI methods even if detected */
>
> static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
> TPACPI_QEC_IBM('1', 'Y', TPACPI_FAN_Q1),
> @@ -8823,6 +8824,9 @@ static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
> TPACPI_Q_LNV3('N', '1', 'O', TPACPI_FAN_NOFAN), /* X1 Tablet (2nd gen) */
> TPACPI_Q_LNV3('R', '0', 'Q', TPACPI_FAN_DECRPM),/* L480 */
> TPACPI_Q_LNV('8', 'F', TPACPI_FAN_TPR), /* ThinkPad x120e */
> + TPACPI_Q_LNV3('R', '0', '0', TPACPI_FAN_NOACPI),/* E560 */
> + TPACPI_Q_LNV3('R', '1', '2', TPACPI_FAN_NOACPI),/* T495 */
> + TPACPI_Q_LNV3('R', '1', '3', TPACPI_FAN_NOACPI),/* T495s */
nit: Please, remove the extra space inside the comment.
> };
>
> static int __init fan_init(struct ibm_init_struct *iibm)
> @@ -8874,6 +8878,16 @@ static int __init fan_init(struct ibm_init_struct *iibm)
> tp_features.fan_ctrl_status_undef = 1;
> }
>
> + if (quirks & TPACPI_FAN_NOACPI) {
> + /* E560, T495, T495s */
> + pr_info("Ignoring buggy ACPI fan access method\n");
> + gfan_handle = NULL;
> + sfan_handle = NULL;
> + fang_handle = NULL;
> + fanw_handle = NULL;
IMO as this fixes a specific commit, only the handles added in that
commit should be nullified here, i.e. only fang and fanw. The others can
be added later if the need arises.
> + fans_handle = NULL;
> + }
> +
> if (gfan_handle) {
> /* 570, 600e/x, 770e, 770x */
> fan_status_access_mode = TPACPI_FAN_RD_ACPI_GFAN;
[1] https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
--
~ Kurt
Powered by blists - more mailing lists