Thanks!
On 2022-05-02 15:12-0400, Mark Pearson wrote:
> Date: Mon, 2 May 2022 15:12:00 -0400
> From: Mark Pearson <markpearson@xxxxxxxxxx>
> To: markpearson@xxxxxxxxxx
> CC: hdegoede@xxxxxxxxxx, markgross@xxxxxxxxxx,
> platform-driver-x86@xxxxxxxxxxxxxxx, lyude@xxxxxxxxxx, thomas@xxxxxxxx
> Subject: [PATCH] platform/x86: thinkpad_acpi: Correct dual fan probe
> X-Mailer: git-send-email 2.35.1
>
> 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;
> --
> 2.35.1
>
Tested-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
Reviewed-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
FYI this now inverts the logic from "platform/x86: thinkpad_acpi: Don't probe 2nd fan if enabled by quirk" [0]
Instead of skipping the probe when there is a quirk the quirk is skipped if the
probe succeeds first.
Going after the rationale in the patch it might be better to turn this around
here, too:
"If we already know that the system in question has a quirk telling us that
the system has a second fan, there's no purpose in probing the second fan -
especially when probing the second fan may not work properly with systems
relying on quirks."
[0] https://lore.kernel.org/platform-driver-x86/20220429211418.4546-3-lyude@xxxxxxxxxx/
