On 2025-01-07 13:29:08-0600, Mario Limonciello wrote: > On 1/7/2025 11:05, Thomas Weißschuh wrote: > > The driver showcases the use of the new subsystem API. > > > > Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx> > > --- > > drivers/platform/x86/Kconfig | 12 ++++ > > drivers/platform/x86/Makefile | 1 + > > drivers/platform/x86/firmware_attributes_test.c | 78 +++++++++++++++++++++++++ > > 3 files changed, 91 insertions(+) > > > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > > index 0258dd879d64be389f4dd9bc309fe089f23cc798..2a0e828657d2f07074944d6c42dc204fc8825a42 100644 > > --- a/drivers/platform/x86/Kconfig > > +++ b/drivers/platform/x86/Kconfig > > @@ -1065,6 +1065,18 @@ source "drivers/platform/x86/x86-android-tablets/Kconfig" > > config FW_ATTR_CLASS > > tristate > > +config FW_ATTR_TEST > > + tristate "Firmware attribute test driver" > > + select FW_ATTR_CLASS > > + help > > + This driver provides a test user of the firmware attribute subsystem. > > + > > + An instance is created at /sys/class/firmware-attributes/test/ > > + container various example attributes. > > + > > + To compile this driver as a module, choose M here: the module > > + will be called firmware_attributes_test. > > + > > I think if you're going to be introducing a test driver it should be > compliant to what's in sysfs-class-firmware-attributes so that when it's > inevitably copy/pasted we end up with higher quality drivers. > > That is you need at a minimum 'type', 'current_value', 'default_value', > 'display_name' and 'display_name_language_code'. Then individual types > employ additional requirements. > > I see 'type', 'current_value', and 'default_value, but I don't see > 'display_name' or 'display_name_language_code' here. > > Furthermore as this is a "string" attribute you're supposed to also have a > "max_length" and "min_length". Agreed that more examples are better. But are these attributes really mandatory? "This attribute is mandatory" is only specified for "type" and "current_value". While "possible_values" certainly looks necessary for "enumeration", "min_length" for "strings" does so much less. Thomas
