diff mbox series

platform/x86: thinkpad_acpi: disable ACPI fan access for T495* and E560

Message ID 20250323030119.17485-1-ImanDevel@gmail.com (mailing list archive)
State Changes Requested, archived
Headers show
Series platform/x86: thinkpad_acpi: disable ACPI fan access for T495* and E560 | expand

Commit Message

Seyediman Seyedarab March 23, 2025, 3:01 a.m. UTC
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@gmail.com>
Co-authored-by: Seyediman Seyedarab <ImanDevel@gmail.com>
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: Seyediman Seyedarab <ImanDevel@gmail.com>
---
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(+)

Comments

Kurt Borja March 24, 2025, 12:24 a.m. UTC | #1
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@gmail.com>

This tag is not required. Instead use Co-developed-by + Signed-off-by
tags, which are required.

> Co-authored-by: Seyediman Seyedarab <ImanDevel@gmail.com>

Move this tag just before your Signed-off-by and change it to
Co-developed-by.

> Reported-by: Vlastimil Holer <vlastimil.holer@gmail.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@vger.kernel.org

> Tested-by: crok <crok.bic@gmail.com>
> Tested-by: Alireza Elikahi <scr0lll0ck1s4b0v3h0m3k3y@gmail.com>
> Signed-off-by: Seyediman Seyedarab <ImanDevel@gmail.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
diff mbox series

Patch

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  */
 };
 
 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;
+		fans_handle = NULL;
+	}
+
 	if (gfan_handle) {
 		/* 570, 600e/x, 770e, 770x */
 		fan_status_access_mode = TPACPI_FAN_RD_ACPI_GFAN;