aibs(4) / atk0110 support for newer systems

[Andriy Gapon]

I've written a patch for aibs(4) (also known as ASUS AI Booster and ATK0110)
that adds support for discovering and querying sensors using newer GGRP ("get
group"?) and GITM ("get item"?) methods:
https://people.freebsd.org/~avg/aibs-ggrp-gitm.diff

If you are using the driver could you please test that the patch does not break
it for you?
If you have an ASUS motherboard where aibs does not find sensors could you
please check if there is any improvement with the patch?
Testing the patch should be as easy as building, installing and loading aibs
module (found in sys/modules/acpi/aibs). Well, also don't forget to apply the
patch with patch -p1 :-)

--
Andriy Gapon
_______________________________________________
freebsd...@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stabl...@freebsd.org"

[Torfinn Ingolfsen]

On Fri, 30 Sep 2016 14:30:02 +0300
Andriy Gapon <a...@FreeBSD.org> wrote:

> Testing the patch should be as easy as building, installing and loading aibs
> module (found in sys/modules/acpi/aibs). Well, also don't forget to apply the
> patch with patch -p1 :-)

The patch doesn't apply cleanly on FreeBSD 9.3-stable:
root@kg-core1# patch -C -p1 < /home/tingo/dl/aibs-ggrp-gitm.diff
Hmm... Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/sys/dev/acpi_support/atk0110.c b/sys/dev/acpi_support/atk0110.c
|index e79a0e00e3577..743684100f737 100644
|--- a/sys/dev/acpi_support/atk0110.c
|+++ b/sys/dev/acpi_support/atk0110.c
--------------------------
Patching file sys/dev/acpi_support/atk0110.c using Plan A...
Hunk #1 succeeded at 28.
Hunk #2 succeeded at 52.
Hunk #3 succeeded at 78.
Hunk #4 succeeded at 124.
Hunk #5 succeeded at 362.
Hunk #6 succeeded at 370.
Hunk #7 failed at 391.
Hunk #8 succeeded at 435.
Hunk #9 succeeded at 450.
Hunk #10 failed at 488.
2 out of 10 hunks failed--saving rejects to sys/dev/acpi_support/atk0110.c.rej
done

I'm running
root@kg-core1# uname -a
FreeBSD kg-core1.kg4.no 9.3-STABLE FreeBSD 9.3-STABLE #3 r304838: Fri Aug 26 12:11:25 CEST 2016 ro...@kg-core1.kg4.no:/usr/obj/usr/src/sys/GENERIC amd64

--
Torfinn Ingolfsen <torfinn....@getmail.no>

[Andriy Gapon]

Please show the .rej file.
Perhaps gpatch --merge=diff3 would work better than the base system patch if the
conflicts are trivial enough.


--
Andriy Gapon

[Andriy Gapon]

On 30/09/2016 19:03, Mark Dixon wrote:
> Hi Andriy,
>
> I tried your patch on an Asus Z170-Deluxe on 11-RELEASEt and aibs did
> not seem to pick up anything before or after. Not 100% what sensors
> this board has.

If
acpidump -d | fgrep ATK0110
doesn't produce any result, then there is no chance of the driver working.
Otherwise I would be interested in a full acpidump -dt output.

[Torfinn Ingolfsen]

On Fri, 30 Sep 2016 19:00:09 +0300

As I used '-C' as an argument to patch, there isn't one:
root@kg-core1# l /sys/dev/acpi_support/atk0110.c.rej
ls: /sys/dev/acpi_support/atk0110.c.rej: No such file or directory



--
Torfinn Ingolfsen <torfinn....@getmail.no>

[Andriy Gapon]

On 30/09/2016 21:59, Torfinn Ingolfsen wrote:
> As I used '-C' as an argument to patch, there isn't one:
> root@kg-core1# l /sys/dev/acpi_support/atk0110.c.rej
> ls: /sys/dev/acpi_support/atk0110.c.rej: No such file or directory

Right. So, I guess I have to specifically ask to not use -C option? :-)

--
Andriy Gapon

[Andriy Gapon]

On 03/10/2016 21:15, Torfinn Ingolfsen wrote:
> Sorry for the delay. I was distracted by other things.
> root@kg-core1# cd /
> root@kg-core1# patch -p1 < /home/tingo/dl/aibs-ggrp-gitm.diff

> Hmm... Looks like a unified diff to me...
> The text leading up to this was:
> --------------------------
> |diff --git a/sys/dev/acpi_support/atk0110.c b/sys/dev/acpi_support/atk0110.c
> |index e79a0e00e3577..743684100f737 100644
> |--- a/sys/dev/acpi_support/atk0110.c
> |+++ b/sys/dev/acpi_support/atk0110.c
> --------------------------
> Patching file sys/dev/acpi_support/atk0110.c using Plan A...
> Hunk #1 succeeded at 28.
> Hunk #2 succeeded at 52.
> Hunk #3 succeeded at 78.
> Hunk #4 succeeded at 124.
> Hunk #5 succeeded at 362.
> Hunk #6 succeeded at 370.
> Hunk #7 failed at 391.
> Hunk #8 succeeded at 435.
> Hunk #9 succeeded at 450.
> Hunk #10 failed at 488.
> 2 out of 10 hunks failed--saving rejects to sys/dev/acpi_support/atk0110.c.rej

> HTH

Yes, it does. Thank you!
It seems like a couple of minor changes are not in the source tree that you are
using. One is some casts in a diagnostic printf and the other is a different
rounding of 0C in Kelvins.
I've generated a patch that should apply to your tree:
https://people.freebsd.org/~avg/aibs-ggrp-gitm.93.diff
Please try.

[Torfinn Ingolfsen]

On Mon, 3 Oct 2016 23:05:48 +0300
Andriy Gapon <a...@FreeBSD.org> wrote:

> Yes, it does. Thank you!
> It seems like a couple of minor changes are not in the source tree that you are
> using. One is some casts in a diagnostic printf and the other is a different
> rounding of 0C in Kelvins.
> I've generated a patch that should apply to your tree:
> https://people.freebsd.org/~avg/aibs-ggrp-gitm.93.diff
> Please try.

The patch applied cleanly (I removed the old one with patch -R first):
root@kg-core1# patch -p0 < /home/tingo/dl/aibs-ggrp-gitm.93.diff

Hmm... Looks like a unified diff to me...
The text leading up to this was:
--------------------------

|Index: sys/dev/acpi_support/atk0110.c
|===================================================================
|--- sys/dev/acpi_support/atk0110.c (revision 306109)
|+++ sys/dev/acpi_support/atk0110.c (working copy)

--------------------------
Patching file sys/dev/acpi_support/atk0110.c using Plan A...
Hunk #1 succeeded at 28.
Hunk #2 succeeded at 52.
Hunk #3 succeeded at 78.

Hunk #4 succeeded at 91.
Hunk #5 succeeded at 124.
Hunk #6 succeeded at 134.
Hunk #7 succeeded at 362.
Hunk #8 succeeded at 370.
Hunk #9 succeeded at 377.
Hunk #10 succeeded at 391.
Hunk #11 succeeded at 435.
Hunk #12 succeeded at 450.
Hunk #13 succeeded at 457.
Hunk #14 succeeded at 488.
Hunk #15 succeeded at 495.
done

However, build fails:
root@kg-core1# pwd
/sys/modules/acpi/aibs
root@kg-core1# make
Warning: Object directory not changed from original /usr/src/sys/modules/acpi/aibs
@ -> /usr/src/sys
machine -> /usr/src/sys/amd64/include
x86 -> /usr/src/sys/x86/include
:> opt_acpi.h
awk -f @/tools/makeobjops.awk @/dev/acpica/acpi_if.m -h
awk -f @/tools/makeobjops.awk @/kern/bus_if.m -h
awk -f @/tools/makeobjops.awk @/kern/device_if.m -h
:> opt_ddb.h
cc -O2 -pipe -fno-strict-aliasing -Werror -D_KERNEL -DKLD_MODULE -nostdinc -I. -I@ -I@/contrib/altq -finline-limit=8000 --param inline-unit-growth=100 --param large-function-growth=1000 -fno-common -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -mcmodel=kernel -mno-red-zone -mno-mmx -mno-sse -msoft-float -fno-asynchronous-unwind-tables -ffreestanding -fstack-protector -std=iso9899:1999 -fstack-protector -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual -Wundef -Wno-pointer-sign -fformat-extensions -Wmissing-include-dirs -fdiagnostics-show-option -c /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c -o atk0110.o
cc1: warnings being treated as errors
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c: In function 'aibs_attach':
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:252: warning: 's_idx' may be used uninitialized in this function
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:252: note: 's_idx' was declared here
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:256: warning: 'so' may be used uninitialized in this function
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:256: note: 'so' was declared here
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:253: warning: 'name' may be used uninitialized in this function
/usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:253: note: 'name' was declared here
*** [atk0110.o] Error code 1

Stop in /usr/src/sys/modules/acpi/aibs.

Do I have to do something more in order to build the new module?
--
Torfinn Ingolfsen <torfinn....@getmail.no>

[Andriy Gapon]

On 05/10/2016 15:37, Torfinn Ingolfsen wrote:
> On Mon, 3 Oct 2016 23:05:48 +0300
> Andriy Gapon <a...@FreeBSD.org> wrote:
>
>> Yes, it does. Thank you!
>> It seems like a couple of minor changes are not in the source tree that you are
>> using. One is some casts in a diagnostic printf and the other is a different
>> rounding of 0C in Kelvins.
>> I've generated a patch that should apply to your tree:
>> https://people.freebsd.org/~avg/aibs-ggrp-gitm.93.diff
>> Please try.
>
> The patch applied cleanly (I removed the old one with patch -R first):

Good!

> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c: In function 'aibs_attach':
> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:252: warning: 's_idx' may be used uninitialized in this function
> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:252: note: 's_idx' was declared here
> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:256: warning: 'so' may be used uninitialized in this function
> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:256: note: 'so' was declared here
> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:253: warning: 'name' may be used uninitialized in this function
> /usr/src/sys/modules/acpi/aibs/../../../dev/acpi_support/atk0110.c:253: note: 'name' was declared here
> *** [atk0110.o] Error code 1
>
> Stop in /usr/src/sys/modules/acpi/aibs.
>
> Do I have to do something more in order to build the new module?
>

Seems like this is a clang vs gcc issue as I didn't get warnings here.
Could you please simply add initialization to those variables in aibs_attach_ggrp()?
E.g.:
s_idx = NULL;
so = NULL;
Thank you!

--
Andriy Gapon

[Andriy Gapon]

On 05/10/2016 23:28, Torfinn Ingolfsen wrote:
> Unfortunately, my machines panics as soon as this module is loaded:

Not good...

> (kgdb) #0 doadump (textdump=<value optimized out>) at pcpu.h:235
> #1 0xffffffff80900b06 in kern_reboot (howto=260)
> at /usr/src/sys/kern/kern_shutdown.c:454
> #2 0xffffffff80901007 in panic (fmt=0x1 <Address 0x1 out of bounds>)
> at /usr/src/sys/kern/kern_shutdown.c:642
> #3 0xffffffff80ce6887 in trap_fatal (frame=0xc, eva=<value optimized out>)
> at /usr/src/sys/amd64/amd64/trap.c:876
> #4 0xffffffff80ce6bd1 in trap_pfault (frame=0xffffff88b8554380, usermode=0)
> at /usr/src/sys/amd64/amd64/trap.c:798
> #5 0xffffffff80ce7199 in trap (frame=0xffffff88b8554380)
> at /usr/src/sys/amd64/amd64/trap.c:462
> #6 0xffffffff80cd0081 in calltrap ()
> at /usr/src/sys/amd64/amd64/exception.S:238
> #7 0xffffffff81bcb078 in aibs_add_sensor () from /boot/kernel/aibs.ko
> #8 0xffffffff81bcb4b4 in aibs_attach_sif () from /boot/kernel/aibs.ko
> #9 0xffffffff81bcb901 in aibs_attach () from /boot/kernel/aibs.ko

I see that line numbers are not reported for the module, so it probably lacks
debug symbols. But it seems that you compile your kernel with them.
Please try to compile the module using
make KERNBUILDDIR=/usr/obj/...


> #10 0xffffffff8093424c in device_attach (dev=0x0) at device_if.h:180
> #11 0xffffffff8033bd43 in acpi_driver_added (dev=<value optimized out>,
> driver=<value optimized out>) at /usr/src/sys/dev/acpica/acpi.c:841
> #12 0xffffffff809323d5 in devclass_driver_added (dc=0xfffffe00093c5900,
> driver=0xffffffff81bcbe9c) at bus_if.h:204
> #13 0xffffffff80932f13 in devclass_add_driver (dc=0xfffffe00093c5900,
> driver=0xffffffff81bcc140, pass=2147483647, dcp=0xffffffff81bcc1e0)
> at /usr/src/sys/kern/subr_bus.c:1086
> #14 0xffffffff808ebef8 in module_register_init (arg=<value optimized out>)
> at /usr/src/sys/kern/kern_module.c:123
> #15 0xffffffff808e38ae in linker_load_module (kldname=<value optimized out>,
> modname=0xfffffe03ec774000 "aibs", parent=0x0, verinfo=0x0,
> lfpp=0xffffff88b8554aa0) at /usr/src/sys/kern/kern_linker.c:233
> #16 0xffffffff808e3fc4 in kern_kldload (td=<value optimized out>,
> file=0xfffffe03ec774000 "aibs", fileid=0xffffff88b8554af4)
> at /usr/src/sys/kern/kern_linker.c:1038
> #17 0xffffffff808e40d4 in sys_kldload (td=0xfffffe020f434490,
> uap=<value optimized out>) at /usr/src/sys/kern/kern_linker.c:1064
> #18 0xffffffff80ce5ffa in amd64_syscall (td=0xfffffe020f434490, traced=0)
> at subr_syscall.c:142
> #19 0xffffffff80cd0367 in Xfast_syscall ()
> at /usr/src/sys/amd64/amd64/exception.S:398
> #20 0x000000080085d10c in ?? ()
> Previous frame inner to this frame (corrupt stack?)
> (kgdb)

[Andriy Gapon]

On 05/10/2016 23:28, Torfinn Ingolfsen wrote:

> On Wed, 5 Oct 2016 16:09:40 +0300
> Andriy Gapon <a...@FreeBSD.org> wrote:
>
>>
>> Seems like this is a clang vs gcc issue as I didn't get warnings here.
>> Could you please simply add initialization to those variables in aibs_attach_ggrp()?
>> E.g.:
>> s_idx = NULL;
>> so = NULL;
>

> I ended up adding

> s_idx = NULL;
> so = NULL;

> name = NULL;
>
> to aibs_attach_ggrp(), that made the module compile.

Okay, thank for letting me know.

> Unfortunately, my machines panics as soon as this module is loaded:

Not good...

> root@kg-core1# cat /var/crash/info.0
> Dump header from device /dev/ada0p3
> Architecture: amd64
> Architecture Version: 2
> Dump Length: 1466040320B (1398 MB)
> Blocksize: 512
> Dumptime: Wed Oct 5 21:48:42 2016
> Hostname: kg-core1.kg4.no
> Magic: FreeBSD Kernel Dump
> Version String: FreeBSD 9.3-STABLE #3 r304838: Fri Aug 26 12:11:25 CEST 2016
> ro...@kg-core1.kg4.no:/usr/obj/usr/src/sys/GENERIC
> Panic String: page fault
> Dump Parity: 1914870814
> Bounds: 0
> Dump Status: good
>
> core.txt.0 attached
> HTH

Seems like the module got built without debug symbols.
Please try
make KERNBUILDDIR=/usr/obj/...

[Andriy Gapon]

On 05/10/2016 23:28, Torfinn Ingolfsen wrote:

> #6 0xffffffff80cd0081 in calltrap ()
> at /usr/src/sys/amd64/amd64/exception.S:238
> #7 0xffffffff81bcb078 in aibs_add_sensor () from /boot/kernel/aibs.ko
> #8 0xffffffff81bcb4b4 in aibs_attach_sif () from /boot/kernel/aibs.ko

Argh, I've just spotted a very silly typo.
Could you please replace '0' with 'o' in
err = aibs_add_sensor(sc, 0, &as[i], &descr);
?
I wish compilers were more noisy about passing a scalar as a pointer argument.

Thanks!

[Andriy Gapon]

On 06/10/2016 00:37, Andriy Gapon wrote:
> On 05/10/2016 23:28, Torfinn Ingolfsen wrote:
>> #6 0xffffffff80cd0081 in calltrap ()
>> at /usr/src/sys/amd64/amd64/exception.S:238
>> #7 0xffffffff81bcb078 in aibs_add_sensor () from /boot/kernel/aibs.ko
>> #8 0xffffffff81bcb4b4 in aibs_attach_sif () from /boot/kernel/aibs.ko
>
> Argh, I've just spotted a very silly typo.
> Could you please replace '0' with 'o' in
> err = aibs_add_sensor(sc, 0, &as[i], &descr);
> ?

Ping.