Re: [PATCH 2/3] ACPI: fan: Ignore broken _FST on Samsung Galaxy Book 930XDB

[Armin Wolf <W_Armin@gmx.de>]

Am 19.10.26 um 16:54 schrieb Leonard Lausen:
> FAN0._FST on the Samsung Galaxy Book 930XDB stores the reference
> returned by Index() in a local variable and passes that reference to
> Add(). ACPICA rejects the operand with AE_AML_OPERAND_TYPE when the fan
> is running. The zero-speed path does not execute the faulty Add() and
> can succeed.

This is very likely connected to 
https://github.com/open-acpica/acpica/issues/908. The ACPI spec says 
that Increment/Decrement is "Equivalent to Add (Addend, 1, Addend)", so 
i suspect that Add() should try to automatically dereference the reference.

I would suggest that you report this issue to the ACPICA guys instead.

Thanks,
Armin Wolf

> The Microsoft fan extensions added a notification handler that evaluates
> _FST on each fan status change. On this machine, fan notifications now
> produce repeated ACPI errors, particularly around s2idle suspend/resume.
> Speed telemetry also fails whenever the faulty path is taken.
> 
> Ignore _FST on this model. This skips the unusable speed telemetry and
> Microsoft notification handling while preserving the legacy firmware fan
> control. Match the vendor and full DMI product string,
> 930XDB/931XDB/930XDY. The firmware was inspected on BIOS
> P11RFX.067.230331.SP.
> 
> Fixes: a5c2fcd82e04 ("ACPI: fan: Add support for Microsoft fan extensions")
> Signed-off-by: Leonard Lausen <leonard@lausen.nl>
> ---
>   drivers/acpi/fan_core.c | 31 +++++++++++++++++++++++++++----
>   1 file changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c
> index 624d0736b581..48a6818e6282 100644
> --- a/drivers/acpi/fan_core.c
> +++ b/drivers/acpi/fan_core.c
> @@ -8,6 +8,7 @@
>    */
>   
>   #include <linux/bits.h>
> +#include <linux/dmi.h>
>   #include <linux/kernel.h>
>   #include <linux/limits.h>
>   #include <linux/math.h>
> @@ -50,6 +51,24 @@ static const struct acpi_device_id fan_device_ids[] = {
>   };
>   MODULE_DEVICE_TABLE(acpi, fan_device_ids);
>   
> +/*
> + * The _FST method on these machines stores an Index() reference in a local
> + * variable and then uses that reference as an Add() operand when the fan is
> + * running.  This aborts the method with AE_AML_OPERAND_TYPE.  The Microsoft
> + * fan extensions install a notification handler that evaluates this broken
> + * method on each fan status change, producing repeated ACPI errors.
> + */
> +static const struct dmi_system_id acpi_fan_broken_fst_dmi_table[] = {
> +	{
> +		.ident = "Samsung Galaxy Book 930XDB/931XDB/930XDY",
> +		.matches = {
> +			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> +			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "930XDB/931XDB/930XDY"),
> +		},
> +	},
> +	{}
> +};
> +
>   /* thermal cooling device callbacks */
>   static int fan_get_max_state(struct thermal_cooling_device *cdev, unsigned long
>   			     *state)
> @@ -511,10 +530,14 @@ static int acpi_fan_probe(struct platform_device *pdev)
>   	platform_set_drvdata(pdev, fan);
>   
>   	if (acpi_has_method(device->handle, "_FST")) {
> -		fan->has_fst = true;
> -		fan->acpi4 = acpi_has_method(device->handle, "_FIF") &&
> -				acpi_has_method(device->handle, "_FPS") &&
> -				acpi_has_method(device->handle, "_FSL");
> +		if (dmi_check_system(acpi_fan_broken_fst_dmi_table)) {
> +			dev_warn(&pdev->dev, FW_BUG "Ignoring broken _FST method\n");
> +		} else {
> +			fan->has_fst = true;
> +			fan->acpi4 = acpi_has_method(device->handle, "_FIF") &&
> +					acpi_has_method(device->handle, "_FPS") &&
> +					acpi_has_method(device->handle, "_FSL");
> +		}
>   	}
>   
>   	if (fan->acpi4) {