On 1/7/2025 14:50, Thomas Weißschuh wrote:
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".
Ah wow, I had thought they were, but you're right they're not!
While "possible_values" certainly looks necessary for "enumeration",
"min_length" for "strings" does so much less.
Even if they're not mandatory, I think it's better to include them for
the same copy/paste reason I mentioned though.
