Re: [PATCH 0/5] platform/x86: introduce asus-bioscfg

[Hans de Goede <hdegoede@redhat.com>]

Hi Luke,

On 7/17/24 4:34 AM, Luke Jones wrote:
> On Wed, 17 Jul 2024, at 3:11 AM, Ilpo Järvinen wrote:
>> On Tue, 16 Jul 2024, Hans de Goede wrote:
>>> On 7/16/24 7:16 AM, Luke D. Jones wrote:
>>>> This is the first major patch I've ever done with the intention of
>>>> introducing a new module, so it's highly likely I've made some mistakes
>>>> or misunderstood something.
>>>>
>>>> TL;DR:
>>>> 1. introduce new module to contain bios attributes, using fw_attributes_class
>>>> 2. deprecate all possible attributes from asus-wmi that were added ad-hoc
>>>> 3. remove those in the next LTS cycle
>>>>
>>>> The idea for this originates from a conversation with Mario Limonciello
>>>> https://lore.kernel.org/platform-driver-x86/371d4109-a3bb-4c3b-802f-4ec27a945c99@amd.com/
>>>>
>>>> It is without a doubt much cleaner to use, easier to discover, and the
>>>> API is well defined as opposed to the random clutter of attributes I had
>>>> been placing in the platform sysfs.
>>>
>>> This is a bit of a novel use of the fw_attributes_class and I'm not
>>> entirely sure of what to think of this.
>>>
>>> The fw_attributes_class API was designed for (mostly enterprise)
>>> x86 machines where it is possible to change all BIOS settings directly
>>> from the OS without entering the BIOS.
>>>
>>> Here some ACPI or WMI function is present to actually enumerate all
>>> the BIOS options (which can be set this way) and get there type.
>>>
>>> IOW there is not a static list of options inside the driver, nor
>>> is there special handling in the driver other then handling differences
>>> per type.
>>>
>>> And if a new BIOS version has new options or a different machine model
>>> has different options then these are discovered automatically.
>>>
>>> This new use is quite different from this. Although I do see that
>>> at least for the attributes using WMI_STORE_INT() + WMI_SHOW_INT()
>>> that there is quite some commonality between some of the attributes.
>>>
>>> I see how using the existing firmware-attributes class API definition
>>> for this, including allow tweaking this with some of the fwupd
>>> firmware-attributes class commandline util work Mario did is a useful
>>> thing to have.
>>>
>>> I guess using the firmware-attributes class for this is ok, but
>>> this _must_ not be named bioscfg, since the existing hp-bioscfg
>>> driver is a "classic" firmware-attributes drivers enumerating all
>>> the options through BIOS provided enumeration functions and I want
>>> the name to make it clear that this is not that. And the Dell
>>> implementation is called dell-wmi-sysman so lets also avoid sysman
>>> as name.
>>>
>>> Maybe call it "asus-bios-tunables" ?   And then if Asus actually
>>> implements some more classic firmware-attributes enumerable interface
>>> we can use "asus-bioscfg" for that.
>>>
>>> Mario, Ilpo what is your opinion on this ?
>>
>> What you suggested sounds good to me.
> 
> Thanks guys. I think there might be a few names that could be suitable
> 
> 1. asus_bios_tuning/tunables
> 2. asus_fw_tuning/tunables
> 3. asus_fw_settings
> 4. asus_armoury
> 
> I'm shying away from the "tuning/tunables" label since there are also a lot of settings which are just toggles for various features rather than actual tunable things. It makes sense in some contexts at least.
> 
> Armoury Crate is the software collection that Asus uses for the gaming laptops, and they tend to lump these settings under that label.
> 
> Personally I'm leaning towards "asus_fw_settings" as it more accurately describes what the module does.

"asus_fw_settings" sounds good to me. I'm looking forward to v2 of this series.

Regards,

Hans