Page MenuHomeFreeBSD

it8613hwm(4): Add new driver for ITE IT8613 hardware monitor
Needs ReviewPublic

Authored by jo_bruelltuete.com on May 5 2023, 12:17 AM.
Tags
None
Referenced Files
F96572034: D39970.diff
Wed, Sep 25, 1:24 PM
F96558055: D39970.diff
Wed, Sep 25, 12:10 PM
F96505075: D39970.diff
Wed, Sep 25, 6:45 AM
F96504956: D39970/new/.diff
Wed, Sep 25, 6:45 AM
Unknown Object (File)
Tue, Sep 24, 3:15 AM
Unknown Object (File)
Tue, Sep 24, 2:46 AM
Unknown Object (File)
Tue, Sep 24, 2:46 AM
Unknown Object (File)
Tue, Sep 24, 2:46 AM
Subscribers
Restricted Owners Package

Details

Reviewers
None
Group Reviewers
Contributor Reviews (src)
Summary

Adds a driver for the hardware monitor bits in the ITE 8613 super io chip.
Right now this supports temperature and fan speed reading.

This is a refresh for D36424.

Test Plan

I've been using this driver for a while now, works fine (but not perfectly... dodgy BIOS).

From /var/log/messages:

May  1 19:16:06 fred kernel: it8613hwm0: <Hardware monitor on ITE SuperIO> at HWM ldn 0x04 on superio0

I'm running stable/13 on this machine but the patch is against current (and thus compile-time tested only).

$ devinfo -r
[...]
        isab0
          isa0
            superio0
                ACPI I/O ports:
                    0x2e-0x2f
              it8613hwm0
                  ACPI I/O ports:
                      0xa35-0xa36
[...]

$ sysctl dev.it8613hwm
dev.it8613hwm.0.fan2: 1480
dev.it8613hwm.0.fan1: 598
dev.it8613hwm.0.fan0: 0
dev.it8613hwm.0.temperature2: 26C
dev.it8613hwm.0.temperature1: 34C
dev.it8613hwm.0.temperature0: 40C
dev.it8613hwm.0.%parent: superio0
dev.it8613hwm.0.%pnpinfo: type=HWM
dev.it8613hwm.0.%location: ldn=0x04
dev.it8613hwm.0.%driver: it8613hwm
dev.it8613hwm.0.%desc: Hardware monitor on ITE SuperIO
dev.it8613hwm.%parent:

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

jo_bruelltuete.com edited the test plan for this revision. (Show Details)
jo_bruelltuete.com edited the test plan for this revision. (Show Details)
jo_bruelltuete.com edited the test plan for this revision. (Show Details)
jo_bruelltuete.com added subscribers: Restricted Owners Package, avg.
jo_bruelltuete.com retitled this revision from Add driver for ITE 8613 hardware monitor to it8613hwm(4): Add new driver for ITE IT8613 hardware monitor.May 8 2023, 3:09 PM

style?

tools/build/checkstyle9.pl ~/D39970.diff

WARNING: This code is highly experimental ... likely isn't a great style(9) match yet

Use of uninitialized value $root in concatenation (.) or string at tools/build/checkstyle9.pl line 1415.
Use of uninitialized value $root in concatenation (.) or string at tools/build/checkstyle9.pl line 1415.
Use of uninitialized value $root in concatenation (.) or string at tools/build/checkstyle9.pl line 1415.
ERROR: "foo*	bar" should be "foo *bar"
#47: FILE: sys/dev/it8613hwm/it8613hwm.c:42:
+	struct resource*	ioport_res;

ERROR: "foo* bar" should be "foo *bar"
#57: FILE: sys/dev/it8613hwm/it8613hwm.c:52:
+	struct it8613hwm_softc* sc = device_get_softc(dev);

ERROR: that open brace { should be on the previous line
#63: FILE: sys/dev/it8613hwm/it8613hwm.c:58:
+	if (tempindex >= 0 && tempindex < 3)
+	{

ERROR: parentheses required on return
#83: FILE: sys/dev/it8613hwm/it8613hwm.c:78:
+	return sysctl_handle_int(oidp, &tempkelvin, 0, req);

ERROR: "foo* bar" should be "foo *bar"
#90: FILE: sys/dev/it8613hwm/it8613hwm.c:85:
+	struct it8613hwm_softc* sc = device_get_softc(dev);

ERROR: that open brace { should be on the previous line
#94: FILE: sys/dev/it8613hwm/it8613hwm.c:89:
+	if (fanindex >= 0 && fanindex < 3)
+	{

ERROR: parentheses required on return
#115: FILE: sys/dev/it8613hwm/it8613hwm.c:110:
+	return sysctl_handle_int(oidp, &fanspeedrpm, 0, req);

ERROR: "foo* bar" should be "foo *bar"
#135: FILE: sys/dev/it8613hwm/it8613hwm.c:130:
+	struct it8613hwm_softc* sc = device_get_softc(dev);

ERROR: "foo* bar" should be "foo *bar"
#136: FILE: sys/dev/it8613hwm/it8613hwm.c:131:
+	struct sysctl_ctx_list* ctx;

ERROR: that open brace { should be on the previous line
#149: FILE: sys/dev/it8613hwm/it8613hwm.c:144:
+	if (error != 0)
+	{

ERROR: that open brace { should be on the previous line
#156: FILE: sys/dev/it8613hwm/it8613hwm.c:151:
+	if (sc->ioport_res == NULL)
+	{

ERROR: that open brace { should be on the previous line
#168: FILE: sys/dev/it8613hwm/it8613hwm.c:163:
+	if (chipid != 0x90)
+	{

ERROR: "foo* bar" should be "foo *bar"
#208: FILE: sys/dev/it8613hwm/it8613hwm.c:203:
+	struct it8613hwm_softc* sc = device_get_softc(dev);

total: 13 errors, 0 warnings, 251 lines checked

Is what my script picks up.

Overall it looks good, though. Just some silly style preferences of the project. I'd like to commit it and the man page companion commit D39971. You might want to run 'igor' on the man page (you'll need to install it as a package) as well as mandoc -T lint. Once those are clean, I'll commit it.

This driver has worked mostly fine for the past year.

There is one (really annoying) glitch (on my machine): the ACPI code for hw.acpi.thermal.tz0.temperature clashes with the temperature reading in this driver. It's not synchronised. So at some point both will try to read from the ITE chip and trample on each other, giving bogus temperature. That then makes the BIOS think the CPU is on fire and starts throttling it to a screatching halt. The machine is alive and fully functional, just really really slow.

I guess I need a mutex with ACPI stuff... any suggestions?

I am not sure that such a OS/ACPI (firmware) mutex is possible.

You may want to consider doing something similar to what they did on Linux with asus_wmi_sensors / asus_wmi_ec_sensors / asus-ec-sensors drivers where they use ACPI interface to query monitoring data instead of going directly to the superio chip.
But I am not sure if it's really possible on your system (its firmware / BIOS).