[PATCH v2] platform/x86: thinkpad_acpi: disable ACPI fan access for T495* and E560

[PATCH v2] platform/x86: thinkpad_acpi: disable ACPI fan access for T495* and E560

[Seyediman Seyedarab]

From: Eduard Christian Dumitrescu <eduard.c.dumitrescu@gmail.com>

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.

Cc: stable@vger.kernel.org
Fixes: 57d0557dfa49 ("platform/x86: thinkpad_acpi: Add Thinkpad Edge E531 fan support")
Reported-by: Vlastimil Holer <vlastimil.holer@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219643
Tested-by: crok <crok.bic@gmail.com>
Tested-by: Alireza Elikahi <scr0lll0ck1s4b0v3h0m3k3y@gmail.com>
Signed-off-by: Eduard Christian Dumitrescu <eduard.c.dumitrescu@gmail.com>
Co-developed-by: Seyediman Seyedarab <ImanDevel@gmail.com>
Signed-off-by: Seyediman Seyedarab <ImanDevel@gmail.com>
---
Changes in v2:
- Added the From: tag for the original author
- Replaced the Co-authored-by tag with Co-developed-by
- Cc'd stable@vger.kernel.org
- Removed the extra space inside the comment
- Dropped nullification of sfan/gfan_handle, as it's unrelated to
  the current fix

Kindest Regards,
Seyediman

 drivers/platform/x86/thinkpad_acpi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index d8df1405edfa..27fd67a2f2d1 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 */
 };
 
 static int __init fan_init(struct ibm_init_struct *iibm)
@@ -8874,6 +8878,13 @@ 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");
+		fang_handle = NULL;
+		fanw_handle = NULL;
+	}
+
 	if (gfan_handle) {
 		/* 570, 600e/x, 770e, 770x */
 		fan_status_access_mode = TPACPI_FAN_RD_ACPI_GFAN;
-- 
2.48.1

Re: [PATCH v2] platform/x86: thinkpad_acpi: disable ACPI fan access for T495* and E560

[Kurt Borja]

On Sun Mar 23, 2025 at 10:29 PM -03, Seyediman Seyedarab wrote:
> From: Eduard Christian Dumitrescu <eduard.c.dumitrescu@gmail.com>
>
> 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.

Reviewed-by: Kurt Borja <kuurtb@gmail.com>

>
> Cc: stable@vger.kernel.org
> Fixes: 57d0557dfa49 ("platform/x86: thinkpad_acpi: Add Thinkpad Edge E531 fan support")
> Reported-by: Vlastimil Holer <vlastimil.holer@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219643
> Tested-by: crok <crok.bic@gmail.com>
> Tested-by: Alireza Elikahi <scr0lll0ck1s4b0v3h0m3k3y@gmail.com>
> Signed-off-by: Eduard Christian Dumitrescu <eduard.c.dumitrescu@gmail.com>
> Co-developed-by: Seyediman Seyedarab <ImanDevel@gmail.com>
> Signed-off-by: Seyediman Seyedarab <ImanDevel@gmail.com>
> ---
> Changes in v2:
> - Added the From: tag for the original author
> - Replaced the Co-authored-by tag with Co-developed-by
> - Cc'd stable@vger.kernel.org
> - Removed the extra space inside the comment
> - Dropped nullification of sfan/gfan_handle, as it's unrelated to
>   the current fix
>
> Kindest Regards,
> Seyediman
>
>  drivers/platform/x86/thinkpad_acpi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index d8df1405edfa..27fd67a2f2d1 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 */
>  };
>  
>  static int __init fan_init(struct ibm_init_struct *iibm)
> @@ -8874,6 +8878,13 @@ 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");
> +		fang_handle = NULL;
> +		fanw_handle = NULL;
> +	}
> +
>  	if (gfan_handle) {
>  		/* 570, 600e/x, 770e, 770x */
>  		fan_status_access_mode = TPACPI_FAN_RD_ACPI_GFAN;

Re: [PATCH v2] platform/x86: thinkpad_acpi: disable ACPI fan access for T495* and E560

[Ilpo Järvinen]

On Sun, 23 Mar 2025, Seyediman Seyedarab wrote:

Hi,

Thanks for looking into reaching conclusion for this issue.

> From: Eduard Christian Dumitrescu <eduard.c.dumitrescu@gmail.com>
> 
> The bug was introduced in commit 57d0557dfa49 ("platform/x86:

the commit

> 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. 

Please put the first paragraph about the commit 57d0557dfa49 here now that 
you've state what the problem/bug is first. You can make them to be one 
joined paragraph.

> 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.

We avoid phrases like "This patch ...", so start with "Add a quirk ..."
Also, this should be before "Fan access ..." sentence to make this 
description more logical (again make them to be part of a same paragraph).

> Cc: stable@vger.kernel.org
> Fixes: 57d0557dfa49 ("platform/x86: thinkpad_acpi: Add Thinkpad Edge E531 fan support")
> Reported-by: Vlastimil Holer <vlastimil.holer@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219643

I'd prefer these tags to be in this order:

Reported-by:
Fixes:
Closes:
Cc: stable@vger.kernel.org

ss it's a bit more logical.

> Tested-by: crok <crok.bic@gmail.com>
> Tested-by: Alireza Elikahi <scr0lll0ck1s4b0v3h0m3k3y@gmail.com>

Did these two person either give you those Tested-by tags or gave 
you green light to add theirs Tested-by tags? I don't see the tags given 
in the bugzilla entry. If they didn't yet, please ask if they're fine with 
you adding their Tested-by tags.

In kernel development, in general, we're more than happy credit people who 
invested their time in testing changes but that being said, we don't 
invent Tested-by tags just from the fact that we know somebody has tested 
a patch.

When respinning v3, please remember to add Kurt's Reviewed-by tag too as 
maintainer's tools only automatically collect them for the most recent 
version so when sending the next version, you need to add them into the 
submission.

> Signed-off-by: Eduard Christian Dumitrescu <eduard.c.dumitrescu@gmail.com>
> Co-developed-by: Seyediman Seyedarab <ImanDevel@gmail.com>
> Signed-off-by: Seyediman Seyedarab <ImanDevel@gmail.com>
> ---
> Changes in v2:
> - Added the From: tag for the original author
> - Replaced the Co-authored-by tag with Co-developed-by
> - Cc'd stable@vger.kernel.org
> - Removed the extra space inside the comment
> - Dropped nullification of sfan/gfan_handle, as it's unrelated to
>   the current fix
> 
> Kindest Regards,
> Seyediman
> 
>  drivers/platform/x86/thinkpad_acpi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index d8df1405edfa..27fd67a2f2d1 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 */
>  };
>  
>  static int __init fan_init(struct ibm_init_struct *iibm)
> @@ -8874,6 +8878,13 @@ 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");
> +		fang_handle = NULL;
> +		fanw_handle = NULL;
> +	}
> +
>  	if (gfan_handle) {
>  		/* 570, 600e/x, 770e, 770x */
>  		fan_status_access_mode = TPACPI_FAN_RD_ACPI_GFAN;
> 

The code change itself looks fine.

-- 
 i.

Re: [PATCH v2] platform/x86: thinkpad_acpi: disable ACPI fan access for T495* and E560

[Seyediman Seyedarab]

Hi,
Thank you for your response.

> > Tested-by: crok <crok.bic@gmail.com>
> > Tested-by: Alireza Elikahi <scr0lll0ck1s4b0v3h0m3k3y@gmail.com>
> 
> Did these two person either give you those Tested-by tags or gave 
> you green light to add theirs Tested-by tags? I don't see the tags given 
> in the bugzilla entry. If they didn't yet, please ask if they're fine with 
> you adding their Tested-by tags.
Alireza Elikahi gave me the green light to add his Tested-by tag
after testing on the E560. I haven't reached out to crok yet, but
I'll send PATCH v3 as soon as I hear back from them and get their
permission.

Kindest Regards,
Seyediman