* Re: [PATCH AUTOSEL 6.1 12/24] platform/x86: thinkpad_acpi: fix for incorrect fan reporting on some ThinkPad systems
2023-12-26 0:22 ` [PATCH AUTOSEL 6.1 12/24] platform/x86: thinkpad_acpi: fix for incorrect fan reporting on some ThinkPad systems Sasha Levin
@ 2024-01-09 11:46 ` Pavel Machek
0 siblings, 0 replies; 2+ messages in thread
From: Pavel Machek @ 2024-01-09 11:46 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-kernel, stable, Vishnu Sankar, Mark Pearson, Hans de Goede,
Ilpo Järvinen, hmh, ibm-acpi-devel, platform-driver-x86
[-- Attachment #1: Type: text/plain, Size: 9515 bytes --]
Hi!
> [ Upstream commit 66e92e23a72761f5b53f970aeb1badc5fd92fc74 ]
>
> 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.
This is just a new feature, and is > 200 lines. We should not have
this in stable.
BR,
Pavel
> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Link: https://lore.kernel.org/r/20231214134702.166464-1-vishnuocv@gmail.com
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> 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 05a55bc31c796..6edd2e294750e 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -8149,8 +8149,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)
> @@ -8158,6 +8169,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 */
>
> @@ -8168,6 +8184,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 {
> @@ -8195,6 +8212,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);
> @@ -8325,6 +8344,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;
> @@ -8341,7 +8369,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);
>
> @@ -8368,7 +8397,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:
> @@ -8380,7 +8415,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) {
> @@ -8396,7 +8431,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:
> @@ -8899,6 +8948,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),
> @@ -8917,6 +8967,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) */
> };
>
> @@ -8951,18 +9003,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 */
> @@ -8971,7 +9032,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 */
> @@ -9146,6 +9208,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);
> @@ -9160,13 +9223,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:
--
People of Russia, stop Putin before his war on Ukraine escalates.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread