Some ThinkPad systems ECFW use non-standard addresses for fan control
and reporting. This patch adds support for such ECFW so that it can report
the correct fan values.
Tested on Thinkpads L13 Yoga Gen 2 and X13 Yoga Gen 2.
Suggested-by: Mark Pearson <[email protected]>
Signed-off-by: Vishnu Sankar <[email protected]>
---
-Improvements to comments as requested.
-Removed the usage of unlikely/likely while reading fan speed.
-Improved and clearer print statements to match the current style.
-Changed seq_puts to seq_printf of an unrelated section of the patch.
-Improved the readability of the code.
-Added more clearer comments.
---
drivers/platform/x86/thinkpad_acpi.c | 98 ++++++++++++++++++++++++----
1 file changed, 85 insertions(+), 13 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index d0b5fd4137bc..dcada766c169 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -7948,8 +7948,19 @@ static struct ibm_struct volume_driver_data = {
* TPACPI_FAN_WR_TPEC is also available and should be used to
* command the fan. The X31/X40/X41 seems to have 8 fan levels,
* but the ACPI tables just mention level 7.
+ *
+ * TPACPI_FAN_RD_TPEC_NS:
+ * This mode is used for a few ThinkPads (L13 Yoga Gen2, X13 Yoga Gen2 etc.)
+ * that are using non-standard EC locations for reporting fan speeds.
+ * Currently these platforms only provide fan rpm reporting.
+ *
*/
+#define FAN_RPM_CAL_CONST 491520 /* FAN RPM calculation offset for some non-standard ECFW */
+
+#define FAN_NS_CTRL_STATUS BIT(2) /* Bit which determines control is enabled or not */
+#define FAN_NS_CTRL BIT(4) /* Bit which determines control is by host or EC */
+
enum { /* Fan control constants */
fan_status_offset = 0x2f, /* EC register 0x2f */
fan_rpm_offset = 0x84, /* EC register 0x84: LSB, 0x85 MSB (RPM)
@@ -7957,6 +7968,11 @@ enum { /* Fan control constants */
fan_select_offset = 0x31, /* EC register 0x31 (Firmware 7M)
bit 0 selects which fan is active */
+ fan_status_offset_ns = 0x93, /* Special status/control offset for non-standard EC Fan1 */
+ fan2_status_offset_ns = 0x96, /* Special status/control offset for non-standard EC Fan2 */
+ fan_rpm_status_ns = 0x95, /* Special offset for Fan1 RPM status for non-standard EC */
+ fan2_rpm_status_ns = 0x98, /* Special offset for Fan2 RPM status for non-standard EC */
+
TP_EC_FAN_FULLSPEED = 0x40, /* EC fan mode: full speed */
TP_EC_FAN_AUTO = 0x80, /* EC fan mode: auto fan control */
@@ -7967,6 +7983,7 @@ enum fan_status_access_mode {
TPACPI_FAN_NONE = 0, /* No fan status or control */
TPACPI_FAN_RD_ACPI_GFAN, /* Use ACPI GFAN */
TPACPI_FAN_RD_TPEC, /* Use ACPI EC regs 0x2f, 0x84-0x85 */
+ TPACPI_FAN_RD_TPEC_NS, /* Use non-standard ACPI EC regs (eg: L13 Yoga gen2 etc.) */
};
enum fan_control_access_mode {
@@ -7994,6 +8011,8 @@ static u8 fan_control_desired_level;
static u8 fan_control_resume_level;
static int fan_watchdog_maxinterval;
+static bool fan_with_ns_addr;
+
static struct mutex fan_mutex;
static void fan_watchdog_fire(struct work_struct *ignored);
@@ -8123,6 +8142,15 @@ static int fan_get_status(u8 *status)
}
break;
+ case TPACPI_FAN_RD_TPEC_NS:
+ /* Default mode is AUTO which means controlled by EC */
+ if (!acpi_ec_read(fan_status_offset_ns, &s))
+ return -EIO;
+
+ if (status)
+ *status = s;
+
+ break;
default:
return -ENXIO;
@@ -8139,7 +8167,8 @@ static int fan_get_status_safe(u8 *status)
if (mutex_lock_killable(&fan_mutex))
return -ERESTARTSYS;
rc = fan_get_status(&s);
- if (!rc)
+ /* NS EC doesn't have register with level settings */
+ if (!rc && !fan_with_ns_addr)
fan_update_desired_level(s);
mutex_unlock(&fan_mutex);
@@ -8166,7 +8195,13 @@ static int fan_get_speed(unsigned int *speed)
if (likely(speed))
*speed = (hi << 8) | lo;
+ break;
+ case TPACPI_FAN_RD_TPEC_NS:
+ if (!acpi_ec_read(fan_rpm_status_ns, &lo))
+ return -EIO;
+ if (speed)
+ *speed = lo ? FAN_RPM_CAL_CONST / lo : 0;
break;
default:
@@ -8178,7 +8213,7 @@ static int fan_get_speed(unsigned int *speed)
static int fan2_get_speed(unsigned int *speed)
{
- u8 hi, lo;
+ u8 hi, lo, status;
bool rc;
switch (fan_status_access_mode) {
@@ -8194,7 +8229,21 @@ static int fan2_get_speed(unsigned int *speed)
if (likely(speed))
*speed = (hi << 8) | lo;
+ break;
+ case TPACPI_FAN_RD_TPEC_NS:
+ rc = !acpi_ec_read(fan2_status_offset_ns, &status);
+ if (rc)
+ return -EIO;
+ if (!(status & FAN_NS_CTRL_STATUS)) {
+ pr_info("secondary fan control not supported\n");
+ return -EIO;
+ }
+ rc = !acpi_ec_read(fan2_rpm_status_ns, &lo);
+ if (rc)
+ return -EIO;
+ if (speed)
+ *speed = lo ? FAN_RPM_CAL_CONST / lo : 0;
break;
default:
@@ -8697,6 +8746,7 @@ static const struct attribute_group fan_driver_attr_group = {
#define TPACPI_FAN_2FAN 0x0002 /* EC 0x31 bit 0 selects fan2 */
#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 */
static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
TPACPI_QEC_IBM('1', 'Y', TPACPI_FAN_Q1),
@@ -8715,6 +8765,8 @@ static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
TPACPI_Q_LNV3('N', '2', 'O', TPACPI_FAN_2CTL), /* P1 / X1 Extreme (2nd gen) */
TPACPI_Q_LNV3('N', '3', '0', TPACPI_FAN_2CTL), /* P15 (1st gen) / P15v (1st gen) */
TPACPI_Q_LNV3('N', '3', '7', TPACPI_FAN_2CTL), /* T15g (2nd gen) */
+ TPACPI_Q_LNV3('R', '1', 'F', TPACPI_FAN_NS), /* L13 Yoga Gen 2 */
+ TPACPI_Q_LNV3('N', '2', 'U', TPACPI_FAN_NS), /* X13 Yoga Gen 2*/
TPACPI_Q_LNV3('N', '1', 'O', TPACPI_FAN_NOFAN), /* X1 Tablet (2nd gen) */
};
@@ -8749,18 +8801,27 @@ static int __init fan_init(struct ibm_init_struct *iibm)
return -ENODEV;
}
+ if (quirks & TPACPI_FAN_NS) {
+ pr_info("ECFW with non-standard fan reg control found\n");
+ fan_with_ns_addr = 1;
+ /* Fan ctrl support from host is undefined for now */
+ tp_features.fan_ctrl_status_undef = 1;
+ }
+
if (gfan_handle) {
/* 570, 600e/x, 770e, 770x */
fan_status_access_mode = TPACPI_FAN_RD_ACPI_GFAN;
} else {
/* all other ThinkPads: note that even old-style
* ThinkPad ECs supports the fan control register */
- if (likely(acpi_ec_read(fan_status_offset,
- &fan_control_initial_status))) {
+ if (fan_with_ns_addr ||
+ likely(acpi_ec_read(fan_status_offset, &fan_control_initial_status))) {
int res;
unsigned int speed;
- fan_status_access_mode = TPACPI_FAN_RD_TPEC;
+ fan_status_access_mode = fan_with_ns_addr ?
+ TPACPI_FAN_RD_TPEC_NS : TPACPI_FAN_RD_TPEC;
+
if (quirks & TPACPI_FAN_Q1)
fan_quirk1_setup();
/* Try and probe the 2nd fan */
@@ -8769,7 +8830,8 @@ static int __init fan_init(struct ibm_init_struct *iibm)
if (res >= 0 && speed != FAN_NOT_PRESENT) {
/* It responded - so let's assume it's there */
tp_features.second_fan = 1;
- tp_features.second_fan_ctl = 1;
+ /* fan control not currently available for ns ECFW */
+ tp_features.second_fan_ctl = !fan_with_ns_addr;
pr_info("secondary fan control detected & enabled\n");
} else {
/* Fan not auto-detected */
@@ -8944,6 +9006,7 @@ static int fan_read(struct seq_file *m)
str_enabled_disabled(status), status);
break;
+ case TPACPI_FAN_RD_TPEC_NS:
case TPACPI_FAN_RD_TPEC:
/* all except 570, 600e/x, 770e, 770x */
rc = fan_get_status_safe(&status);
@@ -8958,13 +9021,22 @@ static int fan_read(struct seq_file *m)
seq_printf(m, "speed:\t\t%d\n", speed);
- if (status & TP_EC_FAN_FULLSPEED)
- /* Disengaged mode takes precedence */
- seq_printf(m, "level:\t\tdisengaged\n");
- else if (status & TP_EC_FAN_AUTO)
- seq_printf(m, "level:\t\tauto\n");
- else
- seq_printf(m, "level:\t\t%d\n", status);
+ if (fan_status_access_mode == TPACPI_FAN_RD_TPEC_NS) {
+ /*
+ * No full speed bit in NS EC
+ * EC Auto mode is set by default.
+ * No other levels settings available
+ */
+ seq_printf(m, "level:\t\t%s\n", status & FAN_NS_CTRL ? "unknown" : "auto");
+ } else {
+ if (status & TP_EC_FAN_FULLSPEED)
+ /* Disengaged mode takes precedence */
+ seq_printf(m, "level:\t\tdisengaged\n");
+ else if (status & TP_EC_FAN_AUTO)
+ seq_printf(m, "level:\t\tauto\n");
+ else
+ seq_printf(m, "level:\t\t%d\n", status);
+ }
break;
case TPACPI_FAN_NONE:
--
2.34.1
On Thu, 14 Dec 2023, Vishnu Sankar wrote:
> Some ThinkPad systems ECFW use non-standard addresses for fan control
> and reporting. This patch adds support for such ECFW so that it can report
> the correct fan values.
> Tested on Thinkpads L13 Yoga Gen 2 and X13 Yoga Gen 2.
>
> Suggested-by: Mark Pearson <[email protected]>
> Signed-off-by: Vishnu Sankar <[email protected]>
> ---
> -Improvements to comments as requested.
> -Removed the usage of unlikely/likely while reading fan speed.
> -Improved and clearer print statements to match the current style.
> -Changed seq_puts to seq_printf of an unrelated section of the patch.
> -Improved the readability of the code.
> -Added more clearer comments.
Nice work, thanks.
Reviewed-by: Ilpo J?rvinen <[email protected]>
--
i.
Hi Ilpo,
Thank you for the review.
Vishnu
On Thu, Dec 14, 2023 at 11:53 PM Ilpo Järvinen
<[email protected]> wrote:
>
> On Thu, 14 Dec 2023, Vishnu Sankar wrote:
>
> > Some ThinkPad systems ECFW use non-standard addresses for fan control
> > and reporting. This patch adds support for such ECFW so that it can report
> > the correct fan values.
> > Tested on Thinkpads L13 Yoga Gen 2 and X13 Yoga Gen 2.
> >
> > Suggested-by: Mark Pearson <[email protected]>
> > Signed-off-by: Vishnu Sankar <[email protected]>
> > ---
> > -Improvements to comments as requested.
> > -Removed the usage of unlikely/likely while reading fan speed.
> > -Improved and clearer print statements to match the current style.
> > -Changed seq_puts to seq_printf of an unrelated section of the patch.
> > -Improved the readability of the code.
> > -Added more clearer comments.
>
> Nice work, thanks.
>
> Reviewed-by: Ilpo Järvinen <[email protected]>
>
>
> --
> i.
--
Regards,
Vishnu Sankar
+817015150407 (Japan)
Hi,
On 12/14/23 15:52, Ilpo Järvinen wrote:
> On Thu, 14 Dec 2023, Vishnu Sankar wrote:
>
>> Some ThinkPad systems ECFW use non-standard addresses for fan control
>> and reporting. This patch adds support for such ECFW so that it can report
>> the correct fan values.
>> Tested on Thinkpads L13 Yoga Gen 2 and X13 Yoga Gen 2.
>>
>> Suggested-by: Mark Pearson <[email protected]>
>> Signed-off-by: Vishnu Sankar <[email protected]>
>> ---
>> -Improvements to comments as requested.
>> -Removed the usage of unlikely/likely while reading fan speed.
>> -Improved and clearer print statements to match the current style.
>> -Changed seq_puts to seq_printf of an unrelated section of the patch.
>> -Improved the readability of the code.
>> -Added more clearer comments.
>
> Nice work, thanks.
>
> Reviewed-by: Ilpo Järvinen <[email protected]>
Ilpo, thank you for the review.
The patch looks good to me too:
Reviewed-by: Hans de Goede <[email protected]>
Ilpo, do you plan on sending out one more fixes pull-req for 6.7
and if yes, can you add this patch to that; or shall I merge
this into for-next so that it gets included in 6.8-rc1 ?
Regards,
Hans
On Thu, 14 Dec 2023 22:47:02 +0900, Vishnu Sankar wrote:
> Some ThinkPad systems ECFW use non-standard addresses for fan control
> and reporting. This patch adds support for such ECFW so that it can report
> the correct fan values.
> Tested on Thinkpads L13 Yoga Gen 2 and X13 Yoga Gen 2.
>
>
Thank you for your contribution, it has been applied to my local
review-ilpo branch. Note it will show up in the public
platform-drivers-x86/review-ilpo branch only once I've pushed my
local branch there, which might take a while.
The list of commits applied:
[1/1] platform/x86: thinkpad_acpi: fix for incorrect fan reporting on some ThinkPad systems
commit: 66e92e23a72761f5b53f970aeb1badc5fd92fc74
--
i.