[PATCH] Thinkpad_acpi: Fix for ThinkPad's with ECFW showing incorrect fan speed

[PATCH] Thinkpad_acpi: Fix for ThinkPad's with ECFW showing incorrect fan speed

[Vishnu Sankar]

Fix for Thinkpad's with ECFW showing incorrect fan speed.
Some models use decimal instead of hexadecimal for the fan speed stored
in the EC registers.
for eg: the rpm register will have 0x4200 instead of 0x1068.
Here the actual RPM is "4200" in decimal.

A quirk added to handle this.

Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
Signed-off-by: Vishnu Sankar <vsankar@lenovo.com>
Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
---
 drivers/platform/x86/thinkpad_acpi.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 4c1b0553f872..6371a9f765c1 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -7936,6 +7936,7 @@ static u8 fan_control_resume_level;
 static int fan_watchdog_maxinterval;
 
 static bool fan_with_ns_addr;
+static bool ecfw_with_fan_dec_rpm;
 
 static struct mutex fan_mutex;
 
@@ -8682,7 +8683,11 @@ static ssize_t fan_fan1_input_show(struct device *dev,
 	if (res < 0)
 		return res;
 
-	return sysfs_emit(buf, "%u\n", speed);
+	/* Check for fan speeds displayed in hexadecimal */
+	if (!ecfw_with_fan_dec_rpm)
+		return sysfs_emit(buf, "%u\n", speed);
+	else
+		return sysfs_emit(buf, "%x\n", speed);
 }
 
 static DEVICE_ATTR(fan1_input, S_IRUGO, fan_fan1_input_show, NULL);
@@ -8699,7 +8704,11 @@ static ssize_t fan_fan2_input_show(struct device *dev,
 	if (res < 0)
 		return res;
 
-	return sysfs_emit(buf, "%u\n", speed);
+	/* Check for fan speeds displayed in hexadecimal */
+	if (!ecfw_with_fan_dec_rpm)
+		return sysfs_emit(buf, "%u\n", speed);
+	else
+		return sysfs_emit(buf, "%x\n", speed);
 }
 
 static DEVICE_ATTR(fan2_input, S_IRUGO, fan_fan2_input_show, NULL);
@@ -8775,6 +8784,7 @@ static const struct attribute_group fan_driver_attr_group = {
 #define TPACPI_FAN_2CTL		0x0004		/* selects fan2 control */
 #define TPACPI_FAN_NOFAN	0x0008		/* no fan available */
 #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 */
 
 static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
 	TPACPI_QEC_IBM('1', 'Y', TPACPI_FAN_Q1),
@@ -8803,6 +8813,7 @@ static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
 	TPACPI_Q_LNV3('R', '1', 'D', TPACPI_FAN_NS),	/* 11e Gen5 GL-R */
 	TPACPI_Q_LNV3('R', '0', 'V', TPACPI_FAN_NS),	/* 11e Gen5 KL-Y */
 	TPACPI_Q_LNV3('N', '1', 'O', TPACPI_FAN_NOFAN),	/* X1 Tablet (2nd gen) */
+	TPACPI_Q_LNV3('R', '0', 'Q', TPACPI_FAN_DECRPM),/* L480 */
 };
 
 static int __init fan_init(struct ibm_init_struct *iibm)
@@ -8847,6 +8858,13 @@ static int __init fan_init(struct ibm_init_struct *iibm)
 		tp_features.fan_ctrl_status_undef = 1;
 	}
 
+	/* Check for the EC/BIOS with RPM reported in decimal*/
+	if (quirks & TPACPI_FAN_DECRPM) {
+		pr_info("ECFW with fan RPM as decimal in EC register\n");
+		ecfw_with_fan_dec_rpm = 1;
+		tp_features.fan_ctrl_status_undef = 1;
+	}
+
 	if (gfan_handle) {
 		/* 570, 600e/x, 770e, 770x */
 		fan_status_access_mode = TPACPI_FAN_RD_ACPI_GFAN;
@@ -9067,7 +9085,11 @@ static int fan_read(struct seq_file *m)
 		if (rc < 0)
 			return rc;
 
-		seq_printf(m, "speed:\t\t%d\n", speed);
+		/* Check for fan speeds displayed in hexadecimal */
+		if (!ecfw_with_fan_dec_rpm)
+			seq_printf(m, "speed:\t\t%d\n", speed);
+		else
+			seq_printf(m, "speed:\t\t%x\n", speed);
 
 		if (fan_status_access_mode == TPACPI_FAN_RD_TPEC_NS) {
 			/*
-- 
2.43.0

Re: [PATCH] Thinkpad_acpi: Fix for ThinkPad's with ECFW showing incorrect fan speed

[Hans de Goede]

Hi,

On 6-Nov-24 12:55 AM, Vishnu Sankar wrote:
> Fix for Thinkpad's with ECFW showing incorrect fan speed.
> Some models use decimal instead of hexadecimal for the fan speed stored
> in the EC registers.
> for eg: the rpm register will have 0x4200 instead of 0x1068.
> Here the actual RPM is "4200" in decimal.
> 
> A quirk added to handle this.

Thank you for your patch/series, I've applied this patch
(series) to my review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

> Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> Signed-off-by: Vishnu Sankar <vsankar@lenovo.com>

Note I have dropped this second duplicate Signed-off-by,
I have kept the first one since that matches the From: 
from this email / the git commit author field.

I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.

Regards,

Hans





> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 4c1b0553f872..6371a9f765c1 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -7936,6 +7936,7 @@ static u8 fan_control_resume_level;
>  static int fan_watchdog_maxinterval;
>  
>  static bool fan_with_ns_addr;
> +static bool ecfw_with_fan_dec_rpm;
>  
>  static struct mutex fan_mutex;
>  
> @@ -8682,7 +8683,11 @@ static ssize_t fan_fan1_input_show(struct device *dev,
>  	if (res < 0)
>  		return res;
>  
> -	return sysfs_emit(buf, "%u\n", speed);
> +	/* Check for fan speeds displayed in hexadecimal */
> +	if (!ecfw_with_fan_dec_rpm)
> +		return sysfs_emit(buf, "%u\n", speed);
> +	else
> +		return sysfs_emit(buf, "%x\n", speed);
>  }
>  
>  static DEVICE_ATTR(fan1_input, S_IRUGO, fan_fan1_input_show, NULL);
> @@ -8699,7 +8704,11 @@ static ssize_t fan_fan2_input_show(struct device *dev,
>  	if (res < 0)
>  		return res;
>  
> -	return sysfs_emit(buf, "%u\n", speed);
> +	/* Check for fan speeds displayed in hexadecimal */
> +	if (!ecfw_with_fan_dec_rpm)
> +		return sysfs_emit(buf, "%u\n", speed);
> +	else
> +		return sysfs_emit(buf, "%x\n", speed);
>  }
>  
>  static DEVICE_ATTR(fan2_input, S_IRUGO, fan_fan2_input_show, NULL);
> @@ -8775,6 +8784,7 @@ static const struct attribute_group fan_driver_attr_group = {
>  #define TPACPI_FAN_2CTL		0x0004		/* selects fan2 control */
>  #define TPACPI_FAN_NOFAN	0x0008		/* no fan available */
>  #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 */
>  
>  static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
>  	TPACPI_QEC_IBM('1', 'Y', TPACPI_FAN_Q1),
> @@ -8803,6 +8813,7 @@ static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
>  	TPACPI_Q_LNV3('R', '1', 'D', TPACPI_FAN_NS),	/* 11e Gen5 GL-R */
>  	TPACPI_Q_LNV3('R', '0', 'V', TPACPI_FAN_NS),	/* 11e Gen5 KL-Y */
>  	TPACPI_Q_LNV3('N', '1', 'O', TPACPI_FAN_NOFAN),	/* X1 Tablet (2nd gen) */
> +	TPACPI_Q_LNV3('R', '0', 'Q', TPACPI_FAN_DECRPM),/* L480 */
>  };
>  
>  static int __init fan_init(struct ibm_init_struct *iibm)
> @@ -8847,6 +8858,13 @@ static int __init fan_init(struct ibm_init_struct *iibm)
>  		tp_features.fan_ctrl_status_undef = 1;
>  	}
>  
> +	/* Check for the EC/BIOS with RPM reported in decimal*/
> +	if (quirks & TPACPI_FAN_DECRPM) {
> +		pr_info("ECFW with fan RPM as decimal in EC register\n");
> +		ecfw_with_fan_dec_rpm = 1;
> +		tp_features.fan_ctrl_status_undef = 1;
> +	}
> +
>  	if (gfan_handle) {
>  		/* 570, 600e/x, 770e, 770x */
>  		fan_status_access_mode = TPACPI_FAN_RD_ACPI_GFAN;
> @@ -9067,7 +9085,11 @@ static int fan_read(struct seq_file *m)
>  		if (rc < 0)
>  			return rc;
>  
> -		seq_printf(m, "speed:\t\t%d\n", speed);
> +		/* Check for fan speeds displayed in hexadecimal */
> +		if (!ecfw_with_fan_dec_rpm)
> +			seq_printf(m, "speed:\t\t%d\n", speed);
> +		else
> +			seq_printf(m, "speed:\t\t%x\n", speed);
>  
>  		if (fan_status_access_mode == TPACPI_FAN_RD_TPEC_NS) {
>  			/*

Re: [PATCH] Thinkpad_acpi: Fix for ThinkPad's with ECFW showing incorrect fan speed

[Vishnu Sankar]

Thanks a lot for accepting this!

On Wed, Nov 6, 2024 at 9:02 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 6-Nov-24 12:55 AM, Vishnu Sankar wrote:
> > Fix for Thinkpad's with ECFW showing incorrect fan speed.
> > Some models use decimal instead of hexadecimal for the fan speed stored
> > in the EC registers.
> > for eg: the rpm register will have 0x4200 instead of 0x1068.
> > Here the actual RPM is "4200" in decimal.
> >
> > A quirk added to handle this.
>
> Thank you for your patch/series, I've applied this patch
> (series) to my review-hans branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>
> > Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> > Signed-off-by: Vishnu Sankar <vsankar@lenovo.com>
>
> Note I have dropped this second duplicate Signed-off-by,
> I have kept the first one since that matches the From:
> from this email / the git commit author field.
Acked.
>
> I will include this patch in my next fixes pull-req to Linus
> for the current kernel development cycle.
Thank you!
>
> Regards,
>
> Hans
>
>
>
>
>
> > Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> > ---
> >  drivers/platform/x86/thinkpad_acpi.c | 28 +++++++++++++++++++++++++---
> >  1 file changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> > index 4c1b0553f872..6371a9f765c1 100644
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -7936,6 +7936,7 @@ static u8 fan_control_resume_level;
> >  static int fan_watchdog_maxinterval;
> >
> >  static bool fan_with_ns_addr;
> > +static bool ecfw_with_fan_dec_rpm;
> >
> >  static struct mutex fan_mutex;
> >
> > @@ -8682,7 +8683,11 @@ static ssize_t fan_fan1_input_show(struct device *dev,
> >       if (res < 0)
> >               return res;
> >
> > -     return sysfs_emit(buf, "%u\n", speed);
> > +     /* Check for fan speeds displayed in hexadecimal */
> > +     if (!ecfw_with_fan_dec_rpm)
> > +             return sysfs_emit(buf, "%u\n", speed);
> > +     else
> > +             return sysfs_emit(buf, "%x\n", speed);
> >  }
> >
> >  static DEVICE_ATTR(fan1_input, S_IRUGO, fan_fan1_input_show, NULL);
> > @@ -8699,7 +8704,11 @@ static ssize_t fan_fan2_input_show(struct device *dev,
> >       if (res < 0)
> >               return res;
> >
> > -     return sysfs_emit(buf, "%u\n", speed);
> > +     /* Check for fan speeds displayed in hexadecimal */
> > +     if (!ecfw_with_fan_dec_rpm)
> > +             return sysfs_emit(buf, "%u\n", speed);
> > +     else
> > +             return sysfs_emit(buf, "%x\n", speed);
> >  }
> >
> >  static DEVICE_ATTR(fan2_input, S_IRUGO, fan_fan2_input_show, NULL);
> > @@ -8775,6 +8784,7 @@ static const struct attribute_group fan_driver_attr_group = {
> >  #define TPACPI_FAN_2CTL              0x0004          /* selects fan2 control */
> >  #define TPACPI_FAN_NOFAN     0x0008          /* no fan available */
> >  #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 */
> >
> >  static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
> >       TPACPI_QEC_IBM('1', 'Y', TPACPI_FAN_Q1),
> > @@ -8803,6 +8813,7 @@ static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
> >       TPACPI_Q_LNV3('R', '1', 'D', TPACPI_FAN_NS),    /* 11e Gen5 GL-R */
> >       TPACPI_Q_LNV3('R', '0', 'V', TPACPI_FAN_NS),    /* 11e Gen5 KL-Y */
> >       TPACPI_Q_LNV3('N', '1', 'O', TPACPI_FAN_NOFAN), /* X1 Tablet (2nd gen) */
> > +     TPACPI_Q_LNV3('R', '0', 'Q', TPACPI_FAN_DECRPM),/* L480 */
> >  };
> >
> >  static int __init fan_init(struct ibm_init_struct *iibm)
> > @@ -8847,6 +8858,13 @@ static int __init fan_init(struct ibm_init_struct *iibm)
> >               tp_features.fan_ctrl_status_undef = 1;
> >       }
> >
> > +     /* Check for the EC/BIOS with RPM reported in decimal*/
> > +     if (quirks & TPACPI_FAN_DECRPM) {
> > +             pr_info("ECFW with fan RPM as decimal in EC register\n");
> > +             ecfw_with_fan_dec_rpm = 1;
> > +             tp_features.fan_ctrl_status_undef = 1;
> > +     }
> > +
> >       if (gfan_handle) {
> >               /* 570, 600e/x, 770e, 770x */
> >               fan_status_access_mode = TPACPI_FAN_RD_ACPI_GFAN;
> > @@ -9067,7 +9085,11 @@ static int fan_read(struct seq_file *m)
> >               if (rc < 0)
> >                       return rc;
> >
> > -             seq_printf(m, "speed:\t\t%d\n", speed);
> > +             /* Check for fan speeds displayed in hexadecimal */
> > +             if (!ecfw_with_fan_dec_rpm)
> > +                     seq_printf(m, "speed:\t\t%d\n", speed);
> > +             else
> > +                     seq_printf(m, "speed:\t\t%x\n", speed);
> >
> >               if (fan_status_access_mode == TPACPI_FAN_RD_TPEC_NS) {
> >                       /*
>


--