| Message ID | 20250222094407.9753-1-josh@joshuagrisham.com (mailing list archive) |
|---|---|
| State | Queued |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | [v4] ACPI: fan: Add fan speed reporting for fans with only _FST | expand |
On Sat, Feb 22, 2025 at 10:44 AM Joshua Grisham <josh@joshuagrisham.com> wrote: > > Add support for ACPI fans with _FST to report their speed even if they do > not support fan control. > > As suggested by Armin Wolf [1] and per the Windows Thermal Management > Design Guide [2], Samsung Galaxy Book series devices (and possibly many > more devices where the Windows guide was strictly followed) only implement > the _FST method and do not support ACPI-based fan control. > > Currently, these fans are not supported by the kernel driver but this patch > will make some very small adjustments to allow them to be supported. > > This patch is tested and working for me on a Samsung Galaxy Book2 Pro whose > DSDT (and several other Samsung Galaxy Book series notebooks which > currently have the same issue) can be found at [3]. > > [1]: https://lore.kernel.org/platform-driver-x86/53c5075b-1967-45d0-937f-463912dd966d@gmx.de > [2]: https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/design-guide > [3]: https://github.com/joshuagrisham/samsung-galaxybook-extras/tree/8e3087a06b8bdcdfdd081367af4b744a56cc4ee9/dsdt > > Signed-off-by: Joshua Grisham <josh@joshuagrisham.com> > Reviewed-by: Armin Wolf <W_Armin@gmx.de> > > --- > > v1->v2: > - Still allow acpi4_only_fst fans to update power state on > suspend/resume > - Fix if / else if logic error > - Also hide hwmon_power_input for acpi4_only_fst fans > > v2->v3: > - Still allow acpi4_only_fst fans to set initial power state on probe > > v3->v4: > - Small tweaks to naming (acpi4_only_fst became has_fst) and logic flow > based on feedback from Rafael > - Built against next and tested working on Samsung Galaxy Book2 Pro > --- > drivers/acpi/fan.h | 1 + > drivers/acpi/fan_attr.c | 37 ++++++++++++++++++++++--------------- > drivers/acpi/fan_core.c | 25 ++++++++++++++++++------- > drivers/acpi/fan_hwmon.c | 8 ++++++++ > 4 files changed, 49 insertions(+), 22 deletions(-) > > diff --git a/drivers/acpi/fan.h b/drivers/acpi/fan.h > index 488b51e2c..15eba1c70 100644 > --- a/drivers/acpi/fan.h > +++ b/drivers/acpi/fan.h > @@ -49,6 +49,7 @@ struct acpi_fan_fst { > > struct acpi_fan { > bool acpi4; > + bool has_fst; > struct acpi_fan_fif fif; > struct acpi_fan_fps *fps; > int fps_count; > diff --git a/drivers/acpi/fan_attr.c b/drivers/acpi/fan_attr.c > index f4f6e2381..22d29ac24 100644 > --- a/drivers/acpi/fan_attr.c > +++ b/drivers/acpi/fan_attr.c > @@ -75,15 +75,6 @@ int acpi_fan_create_attributes(struct acpi_device *device) > struct acpi_fan *fan = acpi_driver_data(device); > int i, status; > > - sysfs_attr_init(&fan->fine_grain_control.attr); > - fan->fine_grain_control.show = show_fine_grain_control; > - fan->fine_grain_control.store = NULL; > - fan->fine_grain_control.attr.name = "fine_grain_control"; > - fan->fine_grain_control.attr.mode = 0444; > - status = sysfs_create_file(&device->dev.kobj, &fan->fine_grain_control.attr); > - if (status) > - return status; > - > /* _FST is present if we are here */ > sysfs_attr_init(&fan->fst_speed.attr); > fan->fst_speed.show = show_fan_speed; > @@ -92,7 +83,19 @@ int acpi_fan_create_attributes(struct acpi_device *device) > fan->fst_speed.attr.mode = 0444; > status = sysfs_create_file(&device->dev.kobj, &fan->fst_speed.attr); > if (status) > - goto rem_fine_grain_attr; > + return status; > + > + if (!fan->acpi4) > + return 0; > + > + sysfs_attr_init(&fan->fine_grain_control.attr); > + fan->fine_grain_control.show = show_fine_grain_control; > + fan->fine_grain_control.store = NULL; > + fan->fine_grain_control.attr.name = "fine_grain_control"; > + fan->fine_grain_control.attr.mode = 0444; > + status = sysfs_create_file(&device->dev.kobj, &fan->fine_grain_control.attr); > + if (status) > + goto rem_fst_attr; > > for (i = 0; i < fan->fps_count; ++i) { > struct acpi_fan_fps *fps = &fan->fps[i]; > @@ -109,18 +112,18 @@ int acpi_fan_create_attributes(struct acpi_device *device) > > for (j = 0; j < i; ++j) > sysfs_remove_file(&device->dev.kobj, &fan->fps[j].dev_attr.attr); > - goto rem_fst_attr; > + goto rem_fine_grain_attr; > } > } > > return 0; > > -rem_fst_attr: > - sysfs_remove_file(&device->dev.kobj, &fan->fst_speed.attr); > - > rem_fine_grain_attr: > sysfs_remove_file(&device->dev.kobj, &fan->fine_grain_control.attr); > > +rem_fst_attr: > + sysfs_remove_file(&device->dev.kobj, &fan->fst_speed.attr); > + > return status; > } > > @@ -129,9 +132,13 @@ void acpi_fan_delete_attributes(struct acpi_device *device) > struct acpi_fan *fan = acpi_driver_data(device); > int i; > > + sysfs_remove_file(&device->dev.kobj, &fan->fst_speed.attr); > + > + if (!fan->acpi4) > + return; > + > for (i = 0; i < fan->fps_count; ++i) > sysfs_remove_file(&device->dev.kobj, &fan->fps[i].dev_attr.attr); > > - sysfs_remove_file(&device->dev.kobj, &fan->fst_speed.attr); > sysfs_remove_file(&device->dev.kobj, &fan->fine_grain_control.attr); > } > diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c > index 10016f52f..8ad12ad3a 100644 > --- a/drivers/acpi/fan_core.c > +++ b/drivers/acpi/fan_core.c > @@ -203,12 +203,16 @@ static const struct thermal_cooling_device_ops fan_cooling_ops = { > * -------------------------------------------------------------------------- > */ > > +static bool acpi_fan_has_fst(struct acpi_device *device) > +{ > + return acpi_has_method(device->handle, "_FST"); > +} > + > static bool acpi_fan_is_acpi4(struct acpi_device *device) > { > return acpi_has_method(device->handle, "_FIF") && > acpi_has_method(device->handle, "_FPS") && > - acpi_has_method(device->handle, "_FSL") && > - acpi_has_method(device->handle, "_FST"); > + acpi_has_method(device->handle, "_FSL"); > } > > static int acpi_fan_get_fif(struct acpi_device *device) > @@ -327,7 +331,12 @@ static int acpi_fan_probe(struct platform_device *pdev) > device->driver_data = fan; > platform_set_drvdata(pdev, fan); > > - if (acpi_fan_is_acpi4(device)) { > + if (acpi_fan_has_fst(device)) { > + fan->has_fst = true; > + fan->acpi4 = acpi_fan_is_acpi4(device); > + } > + > + if (fan->acpi4) { > result = acpi_fan_get_fif(device); > if (result) > return result; > @@ -335,7 +344,9 @@ static int acpi_fan_probe(struct platform_device *pdev) > result = acpi_fan_get_fps(device); > if (result) > return result; > + } > > + if (fan->has_fst) { > result = devm_acpi_fan_create_hwmon(device); > if (result) > return result; > @@ -343,9 +354,9 @@ static int acpi_fan_probe(struct platform_device *pdev) > result = acpi_fan_create_attributes(device); > if (result) > return result; > + } > > - fan->acpi4 = true; > - } else { > + if (!fan->acpi4) { > result = acpi_device_update_power(device, NULL); > if (result) { > dev_err(&device->dev, "Failed to set initial power state\n"); > @@ -391,7 +402,7 @@ static int acpi_fan_probe(struct platform_device *pdev) > err_unregister: > thermal_cooling_device_unregister(cdev); > err_end: > - if (fan->acpi4) > + if (fan->has_fst) > acpi_fan_delete_attributes(device); > > return result; > @@ -401,7 +412,7 @@ static void acpi_fan_remove(struct platform_device *pdev) > { > struct acpi_fan *fan = platform_get_drvdata(pdev); > > - if (fan->acpi4) { > + if (fan->has_fst) { > struct acpi_device *device = ACPI_COMPANION(&pdev->dev); > > acpi_fan_delete_attributes(device); > diff --git a/drivers/acpi/fan_hwmon.c b/drivers/acpi/fan_hwmon.c > index bd0d31a39..e8d906051 100644 > --- a/drivers/acpi/fan_hwmon.c > +++ b/drivers/acpi/fan_hwmon.c > @@ -43,6 +43,10 @@ static umode_t acpi_fan_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_ > case hwmon_fan_input: > return 0444; > case hwmon_fan_target: > + /* Only acpi4 fans support fan control. */ > + if (!fan->acpi4) > + return 0; > + > /* > * When in fine grain control mode, not every fan control value > * has an associated fan performance state. > @@ -57,6 +61,10 @@ static umode_t acpi_fan_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_ > case hwmon_power: > switch (attr) { > case hwmon_power_input: > + /* Only acpi4 fans support fan control. */ > + if (!fan->acpi4) > + return 0; > + > /* > * When in fine grain control mode, not every fan control value > * has an associated fan performance state. > -- Applied as 6.15 material, thanks!
diff --git a/drivers/acpi/fan.h b/drivers/acpi/fan.h index 488b51e2c..15eba1c70 100644 --- a/drivers/acpi/fan.h +++ b/drivers/acpi/fan.h @@ -49,6 +49,7 @@ struct acpi_fan_fst { struct acpi_fan { bool acpi4; + bool has_fst; struct acpi_fan_fif fif; struct acpi_fan_fps *fps; int fps_count; diff --git a/drivers/acpi/fan_attr.c b/drivers/acpi/fan_attr.c index f4f6e2381..22d29ac24 100644 --- a/drivers/acpi/fan_attr.c +++ b/drivers/acpi/fan_attr.c @@ -75,15 +75,6 @@ int acpi_fan_create_attributes(struct acpi_device *device) struct acpi_fan *fan = acpi_driver_data(device); int i, status; - sysfs_attr_init(&fan->fine_grain_control.attr); - fan->fine_grain_control.show = show_fine_grain_control; - fan->fine_grain_control.store = NULL; - fan->fine_grain_control.attr.name = "fine_grain_control"; - fan->fine_grain_control.attr.mode = 0444; - status = sysfs_create_file(&device->dev.kobj, &fan->fine_grain_control.attr); - if (status) - return status; - /* _FST is present if we are here */ sysfs_attr_init(&fan->fst_speed.attr); fan->fst_speed.show = show_fan_speed; @@ -92,7 +83,19 @@ int acpi_fan_create_attributes(struct acpi_device *device) fan->fst_speed.attr.mode = 0444; status = sysfs_create_file(&device->dev.kobj, &fan->fst_speed.attr); if (status) - goto rem_fine_grain_attr; + return status; + + if (!fan->acpi4) + return 0; + + sysfs_attr_init(&fan->fine_grain_control.attr); + fan->fine_grain_control.show = show_fine_grain_control; + fan->fine_grain_control.store = NULL; + fan->fine_grain_control.attr.name = "fine_grain_control"; + fan->fine_grain_control.attr.mode = 0444; + status = sysfs_create_file(&device->dev.kobj, &fan->fine_grain_control.attr); + if (status) + goto rem_fst_attr; for (i = 0; i < fan->fps_count; ++i) { struct acpi_fan_fps *fps = &fan->fps[i]; @@ -109,18 +112,18 @@ int acpi_fan_create_attributes(struct acpi_device *device) for (j = 0; j < i; ++j) sysfs_remove_file(&device->dev.kobj, &fan->fps[j].dev_attr.attr); - goto rem_fst_attr; + goto rem_fine_grain_attr; } } return 0; -rem_fst_attr: - sysfs_remove_file(&device->dev.kobj, &fan->fst_speed.attr); - rem_fine_grain_attr: sysfs_remove_file(&device->dev.kobj, &fan->fine_grain_control.attr); +rem_fst_attr: + sysfs_remove_file(&device->dev.kobj, &fan->fst_speed.attr); + return status; } @@ -129,9 +132,13 @@ void acpi_fan_delete_attributes(struct acpi_device *device) struct acpi_fan *fan = acpi_driver_data(device); int i; + sysfs_remove_file(&device->dev.kobj, &fan->fst_speed.attr); + + if (!fan->acpi4) + return; + for (i = 0; i < fan->fps_count; ++i) sysfs_remove_file(&device->dev.kobj, &fan->fps[i].dev_attr.attr); - sysfs_remove_file(&device->dev.kobj, &fan->fst_speed.attr); sysfs_remove_file(&device->dev.kobj, &fan->fine_grain_control.attr); } diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c index 10016f52f..8ad12ad3a 100644 --- a/drivers/acpi/fan_core.c +++ b/drivers/acpi/fan_core.c @@ -203,12 +203,16 @@ static const struct thermal_cooling_device_ops fan_cooling_ops = { * -------------------------------------------------------------------------- */ +static bool acpi_fan_has_fst(struct acpi_device *device) +{ + return acpi_has_method(device->handle, "_FST"); +} + static bool acpi_fan_is_acpi4(struct acpi_device *device) { return acpi_has_method(device->handle, "_FIF") && acpi_has_method(device->handle, "_FPS") && - acpi_has_method(device->handle, "_FSL") && - acpi_has_method(device->handle, "_FST"); + acpi_has_method(device->handle, "_FSL"); } static int acpi_fan_get_fif(struct acpi_device *device) @@ -327,7 +331,12 @@ static int acpi_fan_probe(struct platform_device *pdev) device->driver_data = fan; platform_set_drvdata(pdev, fan); - if (acpi_fan_is_acpi4(device)) { + if (acpi_fan_has_fst(device)) { + fan->has_fst = true; + fan->acpi4 = acpi_fan_is_acpi4(device); + } + + if (fan->acpi4) { result = acpi_fan_get_fif(device); if (result) return result; @@ -335,7 +344,9 @@ static int acpi_fan_probe(struct platform_device *pdev) result = acpi_fan_get_fps(device); if (result) return result; + } + if (fan->has_fst) { result = devm_acpi_fan_create_hwmon(device); if (result) return result; @@ -343,9 +354,9 @@ static int acpi_fan_probe(struct platform_device *pdev) result = acpi_fan_create_attributes(device); if (result) return result; + } - fan->acpi4 = true; - } else { + if (!fan->acpi4) { result = acpi_device_update_power(device, NULL); if (result) { dev_err(&device->dev, "Failed to set initial power state\n"); @@ -391,7 +402,7 @@ static int acpi_fan_probe(struct platform_device *pdev) err_unregister: thermal_cooling_device_unregister(cdev); err_end: - if (fan->acpi4) + if (fan->has_fst) acpi_fan_delete_attributes(device); return result; @@ -401,7 +412,7 @@ static void acpi_fan_remove(struct platform_device *pdev) { struct acpi_fan *fan = platform_get_drvdata(pdev); - if (fan->acpi4) { + if (fan->has_fst) { struct acpi_device *device = ACPI_COMPANION(&pdev->dev); acpi_fan_delete_attributes(device); diff --git a/drivers/acpi/fan_hwmon.c b/drivers/acpi/fan_hwmon.c index bd0d31a39..e8d906051 100644 --- a/drivers/acpi/fan_hwmon.c +++ b/drivers/acpi/fan_hwmon.c @@ -43,6 +43,10 @@ static umode_t acpi_fan_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_ case hwmon_fan_input: return 0444; case hwmon_fan_target: + /* Only acpi4 fans support fan control. */ + if (!fan->acpi4) + return 0; + /* * When in fine grain control mode, not every fan control value * has an associated fan performance state. @@ -57,6 +61,10 @@ static umode_t acpi_fan_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_ case hwmon_power: switch (attr) { case hwmon_power_input: + /* Only acpi4 fans support fan control. */ + if (!fan->acpi4) + return 0; + /* * When in fine grain control mode, not every fan control value * has an associated fan performance state.