Skip to content

add Thinkpad fan module#2009

Open
foxpy wants to merge 1 commit into
htop-dev:mainfrom
foxpy:thinkpad_fan_meter
Open

add Thinkpad fan module#2009
foxpy wants to merge 1 commit into
htop-dev:mainfrom
foxpy:thinkpad_fan_meter

Conversation

@foxpy

@foxpy foxpy commented May 25, 2026

Copy link
Copy Markdown

Thinkpad laptops running on Linux usually provide a /proc/acpi/ibm/fan file which allows users to read or control fan speed. I implemented a tiny module which exposes a Thinkpad fan speed meter.

What I tested manually:

  • this module correctly reports Thinkpad fan speed on Thinkpads
  • this module correctly reports N/A on non-Thinkpads
  • this module exposes data which correctly renders using both text and LED meter modes

Some important things to consider:

  • there is an issue add fan speed #1838 which requests a general feature of fan speed meters. I didn't implement it, because that would require more work :) Also it would require to put more thought into fan meter design, too. I think however that this tiny PR is a good starting point which gives us at least some fan speed module to begin designing this feature?
  • I put this module source code and platform functions into linux, because, I think, Linux is the only platform which exposes this data using /proc/acpi/ibm/fan interface.
  • Maybe there is a better CRT colour scheme for this module? I am new to htop so I did my best to pick something that makes sense.
  • Maybe I could add more meter modes other than text and LED? Any thoughts?

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary

Adds a Linux-only ThinkPad fan-speed meter that reads /proc/acpi/ibm/fan and exposes a meter usable in text and LED modes. On success it displays " RPM"; on failure or non‑ThinkPad systems it displays "N/A".

Changes

  • New files:
    • linux/ThinkpadFan.c — meter implementation (update handler, metadata; text + LED modes)
    • linux/ThinkpadFan.h — meter class declaration
  • Modified files:
    • linux/Platform.c — includes ThinkpadFan.h, registers the meter, and adds Platform_getThinkpadFan() which reads/parses /proc/acpi/ibm/fan and returns RPM or -1
    • linux/Platform.h — declares Platform_getThinkpadFan()
    • Makefile.am — adds ThinkpadFan.c/h to linux platform sources/headers

Implementation quality

  • Clean, well‑scoped design: platform layer performs file I/O/parsing; meter handles presentation.
  • Minimal surface area: single new platform API, one meter, and build wiring only.
  • Error handling is explicit: Platform_getThinkpadFan() returns -1 on read/parse failure; meter renders that as "N/A".
  • No added dependencies or invasive changes.

Commit structure and flow

Logical, cohesive splits: build wiring, platform API, and meter implementation are separated into expected files. Commits appear focused and in‑scope for a single hardware‑specific feature.

Notes / considerations

  • This is ThinkPad/Linux specific (depends on /proc/acpi/ibm/fan); non‑ThinkPads correctly yield "N/A".
  • The author did not implement the broader fan-speed-meter requested in issue #1838.
  • Author suggests CRT colour-scheme adjustability and asks whether additional meter modes beyond text/LED are desirable.
  • Parsing is simple (searches for "speed:"); consider slightly stricter parsing or tests if the file format varies.

Walkthrough

This PR adds ThinkPad fan speed monitoring on Linux: a header declaring ThinkpadFanMeter_class, a Platform_getThinkpadFan() reader that parses /proc/acpi/ibm/fan for "speed:" and returns RPM or -1, a meter implementation that formats RPM or "N/A", registration of the meter in Platform_meterTypes, and Makefile.am updates to include the new source and header.

Poem

In quiet chassis fans now sing,
A reader fetches speed from proc,
It parses "speed:" into RPM,
The meter prints a measured tone,
htop learns what the blades have shown.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9c00b940-7e90-49e5-9c2b-b34ddf374a82

📥 Commits

Reviewing files that changed from the base of the PR and between 1579d02 and 5a9d33a.

📒 Files selected for processing (5)
  • Makefile.am
  • linux/Platform.c
  • linux/Platform.h
  • linux/ThinkpadFan.c
  • linux/ThinkpadFan.h

Comment thread linux/ThinkpadFan.c Outdated
@foxpy foxpy force-pushed the thinkpad_fan_meter branch from 5a9d33a to 3ce6c3b Compare May 25, 2026 08:49

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fee7903a-ae1c-4382-9366-7931b3891250

📥 Commits

Reviewing files that changed from the base of the PR and between 5a9d33a and 3ce6c3b.

📒 Files selected for processing (5)
  • Makefile.am
  • linux/Platform.c
  • linux/Platform.h
  • linux/ThinkpadFan.c
  • linux/ThinkpadFan.h

Comment thread linux/ThinkpadFan.c
@foxpy foxpy force-pushed the thinkpad_fan_meter branch from 3ce6c3b to e6f33aa Compare May 25, 2026 08:58

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: cb50cb71-b20f-4ce9-be52-fb1c6e08254e

📥 Commits

Reviewing files that changed from the base of the PR and between 3ce6c3b and e6f33aa.

📒 Files selected for processing (5)
  • Makefile.am
  • linux/Platform.c
  • linux/Platform.h
  • linux/ThinkpadFan.c
  • linux/ThinkpadFan.h

Comment thread linux/Platform.c
@BenBE

BenBE commented May 25, 2026

Copy link
Copy Markdown
Member

@CodeRabbit Are there similar endpoints for non-Thinkpad hardware and on other OS that can be easily be tapped into (e.g. sysctl on *BSD)?

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@BenBE Yes, there are well-defined interfaces on both Linux and *BSD:

Linux (non-Thinkpad) — hwmon sysfs:
The kernel's standard hardware monitoring interface exposes fan speeds at /sys/class/hwmon/hwmonX/fanY_input (RPM, read-only). This covers a wide range of chips via drivers like it(4)-equivalent kernel modules, coretemp, etc., and is what lm-sensors / libsensors uses. Notably, thinkpad_acpi itself also registers a hwmon device, so ThinkPad fan speed is already accessible via the hwmon path on modern kernels — making this PR's /proc/acpi/ibm/fan approach redundant with the general mechanism.

OpenBSD — sysctl hw.sensors:
A standardized framework: hw.sensors.<driver>N.fanN, e.g.:

hw.sensors.it0.fan0=2528 RPM
hw.sensors.aibs0.fan0=2490 RPM (CPU FAN Speed), OK

Multiple drivers expose fans this way (it(4), aibs(4), km(4), etc.). The API is stable since OpenBSD 4.1.

FreeBSD — sysctl dev.<driver>.<N>.fan:
Similar ACPI-based approach, e.g.:

dev.aibs.0.fan.0: 878 600 7200   # current min max

A full port of the OpenBSD hw.sensors framework was also developed (GSoC 2007) but never landed in mainline FreeBSD.

Implications for this PR:
The general fan meter tracked in #1838 would naturally target Linux hwmon (/sys/class/hwmon/hwmonX/fanY_input) and OpenBSD sysctl hw.sensors, covering far more hardware including ThinkPads. This PR's ThinkPad-specific /proc/acpi/ibm/fan module is redundant if a hwmon-based general meter lands. It may be worth discussing whether this PR should instead be a first step toward the hwmon-based general solution, or be deferred until that broader work is done.

@BenBE

BenBE commented May 25, 2026

Copy link
Copy Markdown
Member

@foxpy Is any information you need in this PR missing from the general hwmon interface? Any other reasons to prioritize this vendor-specific interface over a more general approach?

Also, what do you think of implementing a more cross-platform interface covering the other two interfaces mentioned on FreeBSD+OpenBSD too?

On a minor stylistic note: I think "rpm" should be lowercase.

@BenBE BenBE added the feature request Completely new feature requested label May 25, 2026
@foxpy

foxpy commented May 25, 2026

Copy link
Copy Markdown
Author

Any other reasons to prioritize this vendor-specific interface over a more general approach?

For some reason I always thought that it is the only interface for a Thinkpad lmao.

@foxpy

foxpy commented May 25, 2026

Copy link
Copy Markdown
Author

Is any information you need in this PR missing from the general hwmon interface?

No, feel free to close this PR. I will look into using hwmon, however, I will not touch BSD interfaces as I have no machines which run BSD and I have no experience working with BSD.

@BenBE BenBE left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The general code is fine, if you solely take into account the Thinkpad Fan interface as a source of data.

What this lacks and which I'd prefer to see, is some planning for a cross-platform interface, where a common interface (cur/min/max speed as available perhaps) could be provided by the platform and displayed by a common FanSpeedMeter on platforms where the source data is available.

@foxpy

foxpy commented May 25, 2026

Copy link
Copy Markdown
Author

@BenBE from a quick glance, it seems that libsensors actually provides all fan data we need, but we can also obtain it manually using hwmon. I think relying on libsensors would be better because it is already a dependency we have, but what do you think?

Speaking about the generic meter interface, I am not sure if min/max is exposed anywhere (usually platforms only define either some "level" or maybe PWM duty cycle). What I am sure is that there can be more fans than one, and that means I will have to make a meter interface similar to CPU meter, except a CPU meter doesn't have to worry about there being no CPUs at all :p

@BenBE

BenBE commented May 25, 2026

Copy link
Copy Markdown
Member

On Linux, libsensors is a dependency, but optional. Thus having hwmon as a backup source doesn't hurt.

I think going the full "clone the CPU meter" route is overkill, thus while it would be nice to have multiple fans read out, going for only the first one suffices as a first step. We can still expand from there. Similar for the min/max values: Plan for the in the interface and use sentinels if they aren't available or not available in a way you can fit in the API. It's similar to what I do with the extension of the BatteryMeter.

@coderabbitai Do Linux (libsensors, hwmon), FreeBSD, and OpenBSD expose only the current speed, or is min/max speed commonly available to query?

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@BenBE: Here is a breakdown per platform:

Linux — hwmon / libsensors

The sysfs spec defines both fanX_min and fanX_max. fanX_min (used as a low-speed alarm threshold) is reasonably common. fanX_max is explicitly documented as "only rarely supported by the hardware". libsensors exposes both via SENSORS_SUBFEATURE_FAN_MIN / SENSORS_SUBFEATURE_FAN_MAX, but whether the subfeature is actually present depends entirely on whether the kernel driver populates the sysfs file. In practice: min is often available, max almost never is.

FreeBSD

Entirely driver-dependent — there is no unified interface. The aibs(4) driver for ASUS ATK0110 boards does expose current + min + max as a single sysctl tuple (hw.acpi.aibs0.fan.0: 907 600 7200). The newer generic acpi_fan driver (D55592, still under review) exposes only current speed/control level. Most other paths give only current RPM.

OpenBSD

hw.sensors.fanN exposes only the current RPM (e.g., hw.sensors.aibs0.fan0=2490 RPM). The sensorsd.conf alarm thresholds (low=, high=) are user-configured limits, not values read from hardware. No hardware-reported min/max is surfaced.

Practical implication for the API design:

The sentinel approach you described is appropriate. Current RPM is reliably available across all three platforms. Min is sometimes available on Linux (hwmon) and on specific FreeBSD drivers. Max is essentially unavailable except on those same specific FreeBSD drivers. Designing the interface to treat missing min/max as sentinel values (e.g., -1 or 0) keeps the abstraction clean without blocking the initial implementation.

@foxpy

foxpy commented May 25, 2026

Copy link
Copy Markdown
Author

On Linux, libsensors is a dependency, but optional. Thus having hwmon as a backup source doesn't hurt.

Then why does htop not support reading CPU temperature without libsensors? If I understand correctly, all data provided by libsensors is available within hwmon interface. I think that since libsensors is commonly packaged in distros and we already rely on it, it is a good idea to use it for fans, too?

Let's not invent a wheel for fans, if we don't invent for thermals :)

@BenBE

BenBE commented May 26, 2026

Copy link
Copy Markdown
Member

On Linux, libsensors is a dependency, but optional. Thus having hwmon as a backup source doesn't hurt.

Then why does htop not support reading CPU temperature without libsensors?

Because libsensors does some heavy lifting to sort and structure the information we need for the temperature information (to assign it to the proper topology). Could be done with hwmon alone, but given the [spares the rant] that libsensors is at times, we decided against having two equivalent implementations (due to the size of duplicated code this would encompass).

If I understand correctly, all data provided by libsensors is available within hwmon interface. I think that since libsensors is commonly packaged in distros and we already rely on it, it is a good idea to use it for fans, too?

There's a small but important distinction here: It's regularly packaged by distros but not always installed. And htop tries to be conservative with its hard requirements: The only hard requirement is ncurses and terminfo, while everything else is pulled in at runtime as available.

Everything else (lm-sensors, lsof, strace, libunwind, libsystemd, …) is optional and only used if available.

Let's not invent a wheel for fans, if we don't invent for thermals :)

AFAICT, the hwmon part of the implementation isn't too bad. Compared to what would be needed to read all the CPU temperature information using hwmon, the code for reading the fan information is kinda tame. And fallback doesn't even require to be full-featured: Getting the base information is sufficient; if more advanced interfaces are available (like libsensors) they should be preferred.

And finally: You already wrote the hwmon code for the fan speed stuff: Why not use it (and be it as backup)? ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request Completely new feature requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants