tech-kern archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: aibs(4): ASUSTeK AI Booster (ACPI ATK0110) hardware monitor with limit support
- To: Paul Goyette <paul%whooppee.com@localhost>
- Subject: Re: aibs(4): ASUSTeK AI Booster (ACPI ATK0110) hardware monitor with limit support
- From: Constantine Aleksandrovich Murenin <C++%Cns.SU@localhost>
- Date: Thu, 31 Dec 2009 17:38:25 -0500
2009/12/31 Paul Goyette <paul%whooppee.com@localhost>:
> On Thu, 31 Dec 2009, Constantine Aleksandrovich Murenin wrote:
>
>>> In the aibs_refresh() routine, you really shouldn't be setting the
>>> sensor state to anything other than SVALID or SINVALID. (Yes, I
>>> know that I did the same thing in acpi_tz, but I need to fix that,
>>> too!)
>>> You've already exported the limit values, so let envsys's code do
>>> the comparisons.
>>>
>>> <snip>
>>
>> Yes, that all makes sense in general, however, it doesn't appear to work
>> in practice: * before applying the attached patch (that no longer overwrites
>> states in refresh) only the user-set limits were ignored and never reported;
>> * after applying the attached patch, neither user-set limits nor driver-set
>> limits are reported.
>> I'm attaching envstat -x -d aibs0 output, too, from which you can see the
>> following: * there's a fan sensor with a 0 speed which is below warning-min;
>> * there's a temperature sensor which is under the user-set warning-min, too.
>> Note that for both of these the states are not set correctly. Perhaps
>> there's some bug in the framework? I'll try to go further into the code
>> sometime later, probably after the New Year.
>
> Definitely a bug in the framework. Silly me, I've made the assumption that
> if a driver can provide initial/default limit values, then it also handles
> the comparing of values - even user-supplied values. This is simply not
> true.
>
> The code in sysmon_envsys_events.c around line 501 currently looks like
>
> if (lims.sel_flags)
> lims.sel_flags |= PROP_DRIVER_LIMITS;
> else
> sed_t->sed_edata->flags &= ~ENVSYS_FMONLIMITS;
>
> It should probably look more like
>
> if (lims.sel_flags == 0)
> sed_t->sed_edata->flags &= ~ENVSYS_FMONLIMITS;
> else if (sed_t->sed_sme->sme_set_limits != NULL)
> lims.sel_flags |= PROP_DRIVER_LIMITS;
>
> This requires that the driver _also_ have a routine to process limit values
> when specified by the user. (If the driver cannot "install" the
> user-provided values, then the *_set_limits() routine needs to clear
> PROP_DRIVER_LIMITS flag - see the sdtemp_set_limits() call in the sdtemp(4)
> driver.)
>
> I'll think this through a little bit more before committing the change.
> (I'll attempt to make the preceeding comment more informative.)
>
>
> BTW, a couple more minor comments on the aibs code.
>
> + case ENVSYS_STEMP:
> + li->sel_critmax = h * 100 * 1000 + 273150000;
> + li->sel_warnmax = l * 100 * 1000 + 273150000;
>
> Most of the NetBSD drivers that I've looked at #define a mnacro to convert
> device values to micro-Kelvins. (Also the conversion in aibs_referesh() ...
No, very few do it by #define, actually, since the conversion doesn't occur all
that often:
http://opengrok.netbsd.org/search?q=273150000&project=%2Fsrc
> + li->sel_flags = PROP_CRITMAX | PROP_WARNMAX;
> + break;
> + case ENVSYS_SFANRPM:
> + /* some boards have strange limits for fans */
> + if (l == 0) {
> + li->sel_warnmin = h;
> + li->sel_flags = PROP_WARNMIN;
> + } else {
> + li->sel_warnmin = l;
> + li->sel_warnmax = h;
> + li->sel_flags = PROP_WARNMIN | PROP_WARNMAX;
> + }
> + break;
>
> Hmmm, is this really correct? If (l == 0) then h is the warnmin value, but
> if (l != 0) then h is warnmax and l is warnmin? That is very strange
> indeed!
Yes, it is. :-) See the comment!
From my original driver for OpenBSD:
bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xf0000 (77 entries)
bios0: vendor Phoenix Technologies, LTD version "ASUS StrikerExtreme ACPI BIOS
Revision 1801" date 10/23/2008
bios0: ASUSTeK Computer INC. StrikerExtreme
...
aibs0 at acpi0
aibs0: TSIF 0: CPUT: 0x06030000 CPU Temperature 900 / 1250 0x10001
aibs0: TSIF 1: MBTP: 0x06030001 MB Temperature 450 / 900 0x10001
aibs0: TSIF 2: ST01: 0x06030021 OPT1 Temperature 900 / 1250 0x10001
aibs0: TSIF 3: ST02: 0x06030022 OPT2 Temperature 900 / 1250 0x10001
aibs0: TSIF 4: ST03: 0x06030023 OPT3 Temperature 900 / 1250 0x10001
aibs0: FSIF: misformed package: 7/8, assume 8
aibs0: FSIF 0: CPUF: 0x06040000 CPU FAN Speed 0 / 1800 0x10001
aibs0: FSIF 1: CHAF: 0x06040001 CHASSIS FAN Speed 0 / 1800 0x1
aibs0: FSIF 2: SF01: 0x06040021 OPT1 FAN Speed 0 / 1800 0x1
aibs0: FSIF 3: SF02: 0x06040022 OPT2 FAN Speed 0 / 1800 0x1
aibs0: FSIF 4: SF03: 0x06040023 OPT3 FAN Speed 0 / 1800 0x1
aibs0: FSIF 5: SF04: 0x06040024 OPT4 FAN Speed 0 / 1800 0x1
aibs0: FSIF 6: SF05: 0x06040025 OPT5 FAN Speed 0 / 1800 0x1
aibs0: FSIF 7: TF03: 0x06040033 PWR FAN Speed 0 / 1800 0x1
aibs0: VSIF 0: VCRE: 0x06020000 Vcore Voltage 850 / 1600 0x1
aibs0: VSIF 1: V333: 0x06020001 +3.3 Voltage 3000 / 3600 0x1
aibs0: VSIF 2: V500: 0x06020002 +5.0 Voltage 4500 / 5500 0x1
aibs0: VSIF 3: V120: 0x06020003 +12.0 Voltage 11200 / 13200 0x1
aibs0: VSIF 4: SV01: 0x06020021 1.2VHT Voltage 1000 / 1400 0x1
aibs0: VSIF 5: SV02: 0x06020022 SB CORE Voltage 1300 / 1700 0x1
aibs0: VSIF 6: SV03: 0x06020023 CPU VTT Voltage 1100 / 1400 0x1
aibs0: VSIF 7: SV04: 0x06020024 DDR2 TERM Voltage 500 / 1300 0x1
aibs0: VSIF 8: SV06: 0x06020026 NB CORE Voltage 1100 / 1600 0x1
aibs0: VSIF 9: SV07: 0x06020027 MEMORY Voltage 1600 / 2500 0x1
But then the newer boards have:
bios0 at mainbus0: SMBIOS rev. 2.5 @ 0xf0720 (50 entries)
bios0: vendor American Megatrends Inc. version "0703" date 02/26/2009
bios0: ASUSTeK Computer INC. P5N64 WS PRO
...
aibs0 at acpi0
aibs0: TSIF 0: \\_SB_.PCI0.SBRG.ASOC.CPUT: 0x06030000 CPU Temperature
600 / 950 0x10001
aibs0: TSIF 1: \\_SB_.PCI0.SBRG.ASOC.MBTP: 0x06030001 MB Temperature
450 / 950 0x10001
aibs0: FSIF 0: \\_SB_.PCI0.SBRG.ASOC.CPUF: 0x06040000 CPU FAN Speed
800 / 7200 0x10001
aibs0: FSIF 1: \\_SB_.PCI0.SBRG.ASOC.CHAF: 0x06040001 CHASSIS FAN Speed
1200 / 7200 0x10001
aibs0: FSIF 2: \\_SB_.PCI0.SBRG.ASOC.CHF2: 0x06040002 CHASSIS FAN 2 Speed
800 / 7200 0x10001
aibs0: FSIF 3: \\_SB_.PCI0.SBRG.ASOC.CHF3: 0x06040003 CHASSIS FAN 3 Speed
800 / 7200 0x10001
aibs0: FSIF 4: \\_SB_.PCI0.SBRG.ASOC.PWRF: 0x06040004 POWER FAN Speed
1800 / 7200 0x10001
aibs0: VSIF 0: \\_SB_.PCI0.SBRG.ASOC.CORV: 0x06020000 Vcore Voltage
850 / 1600 0x1
aibs0: VSIF 1: \\_SB_.PCI0.SBRG.ASOC.V3VV: 0x06020001 +3.3 Voltage
2970 / 3630 0x1
aibs0: VSIF 2: \\_SB_.PCI0.SBRG.ASOC.V5VV: 0x06020002 +5 Voltage
4500 / 5500 0x1
aibs0: VSIF 3: \\_SB_.PCI0.SBRG.ASOC.VV12: 0x06020003 +12 Voltage
10200 / 13800 0x1
> Enjoy your holiday, including tonight's "blue moon". :)
Thank you, you too!
Cheers,
Constantine. http://cnst.su/
--
Constantine A. Murenin
David R. Cheriton School of Computer Science
University of Waterloo
200 University Avenue West
Waterloo, Ontario N2L 3G1
Canada
Home |
Main Index |
Thread Index |
Old Index