Re: [PATCH v6 1/3] firmware: Move firmware attributes class helper

["Derek J. Clark" <derekjohn.clark@gmail.com>]

On July 13, 2026 5:13:15 PM GMT+09:00, Sean Rhodes <sean@starlabs.systems> wrote:
>Move the firmware attributes class helper from drivers/platform/x86 to
>drivers/firmware and expose its class declaration through a public Linux
>header.
>
>The helper is not x86-specific. Keeping it in firmware lets coreboot
>firmware drivers use the standard firmware-attributes ABI without living
>under platform/x86.
>
>Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com>
>Signed-off-by: Sean Rhodes <sean@starlabs.systems>
>---
> MAINTAINERS                                              | 8 ++++++++
> drivers/firmware/Kconfig                                 | 3 +++
> drivers/firmware/Makefile                                | 1 +
> .../x86 => firmware}/firmware_attributes_class.c         | 2 +-
> drivers/platform/x86/Kconfig                             | 3 ---
> drivers/platform/x86/Makefile                            | 2 --
> drivers/platform/x86/asus-armoury.c                      | 2 +-
> drivers/platform/x86/dell/dell-wmi-sysman/sysman.c       | 9 +++++----
> drivers/platform/x86/hp/hp-bioscfg/bioscfg.c             | 9 +++++----
> drivers/platform/x86/lenovo/think-lmi.c                  | 5 +++--
> drivers/platform/x86/lenovo/wmi-other.c                  | 2 +-
> drivers/platform/x86/samsung-galaxybook.c                | 3 ++-
> .../linux/firmware_attributes.h                          | 6 +++---
> 13 files changed, 33 insertions(+), 22 deletions(-)
> rename drivers/{platform/x86 => firmware}/firmware_attributes_class.c (94%)
> rename drivers/platform/x86/firmware_attributes_class.h => include/linux/firmware_attributes.h (60%)
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index f37a81950e25..07bdba98b1b1 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -10114,6 +10114,14 @@ F:	include/linux/firewire.h
> F:	include/uapi/linux/firewire*.h
> F:	tools/firewire/
> 
>+FIRMWARE ATTRIBUTES CLASS
>+M:	Hans de Goede <hansg@kernel.org>
>+M:	Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>+L:	platform-driver-x86@vger.kernel.org
>+S:	Maintained
>+F:	drivers/firmware/firmware_attributes_class.c
>+F:	include/linux/firmware_attributes.h
>+
> FIRMWARE FRAMEWORK FOR ARMV8-A
> M:	Sudeep Holla <sudeep.holla@kernel.org>
> L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
>index 12dc70254842..78a239f8bbef 100644
>--- a/drivers/firmware/Kconfig
>+++ b/drivers/firmware/Kconfig
>@@ -8,6 +8,9 @@ menu "Firmware Drivers"
> 
> source "drivers/firmware/arm_scmi/Kconfig"
> 
>+config FW_ATTR_CLASS
>+	tristate
>+
> config ARM_SCPI_PROTOCOL
> 	tristate "ARM System Control and Power Interface (SCPI) Message Protocol"
> 	depends on ARM || ARM64 || COMPILE_TEST
>diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
>index 4ddec2820c96..385b6e8b7fc6 100644
>--- a/drivers/firmware/Makefile
>+++ b/drivers/firmware/Makefile
>@@ -16,6 +16,7 @@ obj-$(CONFIG_FIRMWARE_MEMMAP)	+= memmap.o
> obj-$(CONFIG_MTK_ADSP_IPC)	+= mtk-adsp-ipc.o
> obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
> obj-$(CONFIG_FW_CFG_SYSFS)	+= qemu_fw_cfg.o
>+obj-$(CONFIG_FW_ATTR_CLASS)	+= firmware_attributes_class.o
> obj-$(CONFIG_SYSFB)		+= sysfb.o
> obj-$(CONFIG_SYSFB_SIMPLEFB)	+= sysfb_simplefb.o
> obj-$(CONFIG_TH1520_AON_PROTOCOL) += thead,th1520-aon.o
>diff --git a/drivers/platform/x86/firmware_attributes_class.c b/drivers/firmware/firmware_attributes_class.c
>similarity index 94%
>rename from drivers/platform/x86/firmware_attributes_class.c
>rename to drivers/firmware/firmware_attributes_class.c
>index 736e96c186d9..503ecc288086 100644
>--- a/drivers/platform/x86/firmware_attributes_class.c
>+++ b/drivers/firmware/firmware_attributes_class.c
>@@ -3,7 +3,7 @@
> /* Firmware attributes class helper module */
> 
> #include <linux/module.h>
>-#include "firmware_attributes_class.h"
>+#include <linux/firmware_attributes.h>
> 
> const struct class firmware_attributes_class = {
> 	.name = "firmware-attributes",
>diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>index b54b5212b204..beed14c38b54 100644
>--- a/drivers/platform/x86/Kconfig
>+++ b/drivers/platform/x86/Kconfig
>@@ -926,9 +926,6 @@ config DASHARO_ACPI
> 
> source "drivers/platform/x86/x86-android-tablets/Kconfig"
> 
>-config FW_ATTR_CLASS
>-	tristate
>-
> config INTEL_IMR
> 	bool "Intel Isolated Memory Region support"
> 	depends on X86_INTEL_QUARK && IOSF_MBI
>diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>index 872ac3842391..cab20a4145e3 100644
>--- a/drivers/platform/x86/Makefile
>+++ b/drivers/platform/x86/Makefile
>@@ -63,8 +63,6 @@ obj-$(CONFIG_X86_PLATFORM_DRIVERS_HP)	+= hp/
> # Hewlett Packard Enterprise
> obj-$(CONFIG_UV_SYSFS)       += uv_sysfs.o
> 
>-obj-$(CONFIG_FW_ATTR_CLASS)	+= firmware_attributes_class.o
>-
> # IBM Thinkpad (before 2005)
> obj-$(CONFIG_IBM_RTL)		+= ibm_rtl.o
> obj-$(CONFIG_SENSORS_HDAPS)	+= hdaps.o
>diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
>index 495dc1e31d40..e6af93608ec5 100644
>--- a/drivers/platform/x86/asus-armoury.c
>+++ b/drivers/platform/x86/asus-armoury.c
>@@ -20,6 +20,7 @@
> #include <linux/dmi.h>
> #include <linux/err.h>
> #include <linux/errno.h>
>+#include <linux/firmware_attributes.h>
> #include <linux/fs.h>
> #include <linux/kernel.h>
> #include <linux/kmod.h>
>@@ -34,7 +35,6 @@
> #include <linux/sysfs.h>
> 
> #include "asus-armoury.h"
>-#include "firmware_attributes_class.h"
> 
> #define ASUS_NB_WMI_EVENT_GUID "0B3CBB35-E3C2-45ED-91C2-4C5A6D195D1C"
> 
>diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
>index ab46a023cc34..963b3473991d 100644
>--- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
>+++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
>@@ -8,18 +8,19 @@
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> #include <linux/align.h>
>-#include <linux/fs.h>
> #include <linux/dmi.h>
>-#include <linux/module.h>
>+#include <linux/firmware_attributes.h>
>+#include <linux/fs.h>
> #include <linux/kernel.h>
>+#include <linux/module.h>
>+#include <linux/nls.h>
> #include <linux/string.h>
> #include <linux/sysfs.h>
> #include <linux/wmi.h>
>+
> #include "dell-wmi-sysman.h"
>-#include "../../firmware_attributes_class.h"
> 
> #define MAX_TYPES  4
>-#include <linux/nls.h>
> 
> struct wmi_sysman_priv wmi_priv = {
> 	.mutex = __MUTEX_INITIALIZER(wmi_priv.mutex),
>diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
>index 27fd6cd21529..f13f21580acd 100644
>--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
>+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
>@@ -7,16 +7,17 @@
> 
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
>+#include <linux/errno.h>
>+#include <linux/firmware_attributes.h>
> #include <linux/fs.h>
>-#include <linux/module.h>
> #include <linux/kernel.h>
>+#include <linux/module.h>
>+#include <linux/nls.h>
> #include <linux/printk.h>
> #include <linux/string.h>
> #include <linux/wmi.h>
>+
> #include "bioscfg.h"
>-#include "../../firmware_attributes_class.h"
>-#include <linux/nls.h>
>-#include <linux/errno.h>
> 

I wasn't aware so many drivers had headers out of the usual order. There may be a reason these (and others) were in that specific order, and the changes aren't really related to the problem you're looking at. It would probably be better to not touch the load order of anything except the firmware_attributes header, but wait for Armin or Ilpo to provide more specific feedback on that. If they don't mind it you'll at least want to add some documentation in the commit message that it was changed. Something like:

While updating includes, ensure existing includes are alphabetical order.

Generally though, Ilpo will request those sorts of changes be in their own patch.

Cheers,
Derek

> MODULE_AUTHOR("Jorge Lopez <jorge.lopez2@hp.com>");
> MODULE_DESCRIPTION("HP BIOS Configuration Driver");
>diff --git a/drivers/platform/x86/lenovo/think-lmi.c b/drivers/platform/x86/lenovo/think-lmi.c
>index e215e86e3db7..4437785aa1f2 100644
>--- a/drivers/platform/x86/lenovo/think-lmi.c
>+++ b/drivers/platform/x86/lenovo/think-lmi.c
>@@ -13,14 +13,15 @@
> 
> #include <linux/acpi.h>
> #include <linux/array_size.h>
>+#include <linux/dmi.h>
> #include <linux/errno.h>
>+#include <linux/firmware_attributes.h>
> #include <linux/fs.h>
> #include <linux/mutex.h>
> #include <linux/string_helpers.h>
> #include <linux/types.h>
>-#include <linux/dmi.h>
> #include <linux/wmi.h>
>-#include "../firmware_attributes_class.h"
>+
> #include "think-lmi.h"
> 
> static bool debug_support;
>diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>index fbb32bf404f2..b175971bb308 100644
>--- a/drivers/platform/x86/lenovo/wmi-other.c
>+++ b/drivers/platform/x86/lenovo/wmi-other.c
>@@ -33,6 +33,7 @@
> #include <linux/container_of.h>
> #include <linux/device.h>
> #include <linux/export.h>
>+#include <linux/firmware_attributes.h>
> #include <linux/gfp_types.h>
> #include <linux/hwmon.h>
> #include <linux/idr.h>
>@@ -50,7 +51,6 @@
> #include "wmi-capdata.h"
> #include "wmi-events.h"
> #include "wmi-helpers.h"
>-#include "../firmware_attributes_class.h"
> 
> #define LENOVO_OTHER_MODE_GUID "DC2A8805-3A8C-41BA-A6F7-092E0089CD3B"
> 
>diff --git a/drivers/platform/x86/samsung-galaxybook.c b/drivers/platform/x86/samsung-galaxybook.c
>index 6382af0b106c..3911f60c23ea 100644
>--- a/drivers/platform/x86/samsung-galaxybook.c
>+++ b/drivers/platform/x86/samsung-galaxybook.c
>@@ -14,6 +14,7 @@
> #include <linux/acpi.h>
> #include <linux/bits.h>
> #include <linux/err.h>
>+#include <linux/firmware_attributes.h>
> #include <linux/i8042.h>
> #include <linux/init.h>
> #include <linux/input.h>
>@@ -27,8 +28,8 @@
> #include <linux/sysfs.h>
> #include <linux/uuid.h>
> #include <linux/workqueue.h>
>+
> #include <acpi/battery.h>
>-#include "firmware_attributes_class.h"
> 
> #define DRIVER_NAME "samsung-galaxybook"
> 
>diff --git a/drivers/platform/x86/firmware_attributes_class.h b/include/linux/firmware_attributes.h
>similarity index 60%
>rename from drivers/platform/x86/firmware_attributes_class.h
>rename to include/linux/firmware_attributes.h
>index d27abe54fcf9..d4026ba1fcc2 100644
>--- a/drivers/platform/x86/firmware_attributes_class.h
>+++ b/include/linux/firmware_attributes.h
>@@ -2,11 +2,11 @@
> 
> /* Firmware attributes class helper module */
> 
>-#ifndef FW_ATTR_CLASS_H
>-#define FW_ATTR_CLASS_H
>+#ifndef _LINUX_FIRMWARE_ATTRIBUTES_H
>+#define _LINUX_FIRMWARE_ATTRIBUTES_H
> 
> #include <linux/device/class.h>
> 
> extern const struct class firmware_attributes_class;
> 
>-#endif /* FW_ATTR_CLASS_H */
>+#endif /* _LINUX_FIRMWARE_ATTRIBUTES_H */