Seems to work great on my machine! The one thing it's missing is a Fixes: tag
for the commit that introduced the dual fan probing code originally. With that
fixed:
Reviewed-by: Lyude Paul <lyude@xxxxxxxxxx>
Tested-by: Lyude Paul <lyude@xxxxxxxxxx>
On Mon, 2022-05-02 at 15:12 -0400, Mark Pearson wrote:
> There was an issue with the dual fan probe whereby the probe was
> failing as it assuming that second_fan support was not available.
>
> Corrected the logic so the probe works correctly. Cleaned up so
> quirks only used if 2nd fan not detected.
>
> Tested on X1 Carbon 10 (2 fans), X1 Carbon 9 (2 fans) and T490 (1 fan)
>
> Signed-off-by: Mark Pearson <markpearson@xxxxxxxxxx>
> ---
> drivers/platform/x86/thinkpad_acpi.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> b/drivers/platform/x86/thinkpad_acpi.c
> index f385450af864..5eea6651a312 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -8862,24 +8862,27 @@ static int __init fan_init(struct ibm_init_struct
> *iibm)
> fan_status_access_mode = TPACPI_FAN_RD_TPEC;
> if (quirks & TPACPI_FAN_Q1)
> fan_quirk1_setup();
> - if (quirks & TPACPI_FAN_2FAN) {
> - tp_features.second_fan = 1;
> - pr_info("secondary fan support enabled\n");
> - }
> - if (quirks & TPACPI_FAN_2CTL) {
> - tp_features.second_fan = 1;
> - tp_features.second_fan_ctl = 1;
> - pr_info("secondary fan control enabled\n");
> - }
> /* Try and probe the 2nd fan */
> + tp_features.second_fan = 1; /* needed for get_speed
> to work */
> res = fan2_get_speed(&speed);
> if (res >= 0) {
> /* It responded - so let's assume it's there
> */
> tp_features.second_fan = 1;
> tp_features.second_fan_ctl = 1;
> pr_info("secondary fan control detected &
> enabled\n");
> + } else {
> + /* Fan not auto-detected */
> + tp_features.second_fan = 0;
> + if (quirks & TPACPI_FAN_2FAN) {
> + tp_features.second_fan = 1;
> + pr_info("secondary fan support
> enabled\n");
> + }
> + if (quirks & TPACPI_FAN_2CTL) {
> + tp_features.second_fan = 1;
> + tp_features.second_fan_ctl = 1;
> + pr_info("secondary fan control
> enabled\n");
> + }
> }
> -
> } else {
> pr_err("ThinkPad ACPI EC access misbehaving, fan
> status and control unavailable\n");
> return -ENODEV;
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
