Messages in this thread | ![/](/images/icornerl.gif) | | Date | Sat, 13 Apr 2024 01:18:51 +0200 | Subject | Re: [PATCH v3 2/2] ACPI: fan: Add hwmon support | From | Armin Wolf <> |
| |
Am 12.04.24 um 23:01 schrieb Guenter Roeck:
> On Fri, Apr 12, 2024 at 10:27:56PM +0200, Armin Wolf wrote: >>>> + case hwmon_fan_fault: >>>> + *val = (fst.speed == FAN_SPEED_UNAVAILABLE); >>> Is it documented that this is indeed a fault (broken fan) ? >>> >> Hi, >> >> it actually means that the fan does not support speed reporting. >> >>>> + return 0; >>>> + default: >>>> + break; >>>> + } >>>> + break; >>>> + case hwmon_power: >>>> + fps = acpi_fan_get_current_fps(fan, fst.control); >>>> + if (!fps) >>>> + return -ENODATA; >>>> + >>>> + switch (attr) { >>>> + case hwmon_power_input: >>>> + if (fps->power == FAN_POWER_UNAVAILABLE) >>>> + return -ENODATA; >>>> + >>>> + if (fps->power > LONG_MAX / MICROWATT_PER_MILLIWATT) >>>> + return -EOVERFLOW; >>>> + >>>> + *val = fps->power * MICROWATT_PER_MILLIWATT; >>>> + return 0; >>>> + case hwmon_power_fault: >>>> + *val = (fps->power == FAN_POWER_UNAVAILABLE); >>> Is it documented that this is indeed a power supply failure ? >>> What if the value is simply not reported ? "UNAVAILABLE" is not >>> commonly associated with a "fault". >>> >>> Guenter >>> >> FAN_POWER_UNAVAILABLE signals that the power value is not supported. >> Would it be more suitable to drop the fault attributes and just return -ENODATA in such a case? >> > There should be no fault attributes unless a real fault > is reported, and if power reporting is not supported the > hwmon_power_input attribute should not even be created. > > The same really applies to the fan speed atribute: If reading > the fan speed is not supported, the attribute should not even > exist. > > Guenter > I see, however it seems that some ACPI implementations also return a fan speed of 0xffffffff to signal an error, so we cannot use this value to check if the BIOS supports fan speed reporting.
I will send a v4 patch witch will drop the fault attributes. When encountering a fan speed of 0xffffffff, returning -ENODATA should be ok i think.
Thanks, Armin Wolf
| ![\](/images/icornerr.gif) |