linux-kernel - Re: [PATCH v6 0/1] asus-wmi: Add support for custom fan curves

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2af6628e-118f-6a75-8074-2f4144c7f8e7@roeck-us.net>
Date:   Sun, 29 Aug 2021 08:18:01 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Luke Jones <luke@...nes.dev>, Hans de Goede <hdegoede@...hat.com>
Cc:     linux-kernel@...r.kernel.org, pobrn@...tonmail.com,
        platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v6 0/1] asus-wmi: Add support for custom fan curves

On 8/29/21 3:03 AM, Luke Jones wrote:
> 
> 
> On Sun, Aug 29 2021 at 11:57:55 +0200, Hans de Goede <hdegoede@...hat.com> wrote:
>> Hi Luke,
>>
>> On 8/29/21 9:14 AM, Luke D. Jones wrote:
>>>  Add support for custom fan curves found on some ASUS ROG laptops.
>>>
>>>  - V1
>>>    + Initial patch work
>>>  - V2
>>>    + Don't fail and remove wmi driver if error from
>>>      asus_wmi_evaluate_method_buf() if error is -ENODEV
>>>  - V3
>>>    + Store the "default" fan curves
>>>    + Call throttle_thermal_policy_write() if a curve is erased to ensure
>>>      that the factory default for a profile is applied again
>>>  - V4
>>>    + Do not apply default curves by default. Testers have found that the
>>>      default curves don't quite match actual no-curve behaviours
>>>    + Add method to enable/disable curves for each profile
>>>  - V5
>>>    + Remove an unrequired function left over from previous iterations
>>>    + Ensure default curves are applied if user writes " " to a curve path
>>>    + Rename "active_fan_curve_profiles" to "enabled_fan_curve_profiles" to
>>>      better reflect the behavious of this setting
>>>    + Move throttle_thermal_policy_write_*pu_curves() and rename to
>>>      fan_curve_*pu_write()
>>>    + Merge fan_curve_check_valid() and fan_curve_write()
>>>    + Remove some leftover debug statements
>>>  - V6
>>>    + Refactor data structs to store  array or u8 instead of strings.
>>>      This affects the entire patch except the enabled_fan_curves block
>>>    + Use sysfs_match_string in enabled_fan_curve block
>>>    + Add some extra comments to describe things
>>>    + Allow some variation in how fan curve input can be formatted
>>>    + Use SENSOR_DEVICE_ATTR_2_RW() to reduce the amount of lines per
>>>      fan+profile combo drastically
>>
>> Thank you for your continued work on this. I read in the discussin of v5
>> that you discussed using the standard auto_point#_pwm, auto_point#_temp
>> pairs. I see here that you have decided to not go that route, may I ask
>> why ?
> 
> Sure, primary reason is because the RPM for the fans is in percentage so it didn't really make sense to me to use that format.
> 
> Also if the max for that is 255 then I'd need to introduce scaling to make match what the ACPI method expects (max 100). But yeah, auto_point#_pwm changes the meaning.
> 

Bad argument. That is true for other controllers as well. You could
just scale it up and down as needed.

The whole point of an ABI is that it is standardized.
If others would [be permitted to] follow your line of argument,
we would not have a useful ABI because "my chip provides/needs
data in some other format".

Guenter