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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

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.

-- 
 i.