* [PATCH v7 0/3] firmware: Add coreboot CFR firmware attributes driver
@ 2026-07-17 8:50 Sean Rhodes
2026-07-17 8:50 ` [PATCH v7 1/3] firmware: Move firmware attributes class helper Sean Rhodes
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Sean Rhodes @ 2026-07-17 8:50 UTC (permalink / raw)
To: tzungbi
Cc: hansg, ilpo.jarvinen, corentin.chary, luke, denis.benato,
prasanth.ksr, jorge.lopez2, mpearson-lenovo, derekjohn.clark,
josh, briannorris, jwerner, tzimmermann, javierm, kees,
u.kleine-koenig, mst, chenhuacai, wenst, florian.fainelli,
titouan.ameline, oliver, linux-kernel, platform-driver-x86,
Dell.Client.Kernel, chrome-platform, Sean Rhodes
Move the firmware attributes class helper out of platform/x86, rename
the coreboot-table firmware driver directory from google to coreboot, and
add a coreboot CFR firmware attributes driver.
Changes in v7:
- Restore the existing include order in converted platform/x86 drivers
and add Oliver's Tested-by to patch 1.
- Use for loops for the CFR record walks, document pointer-based value
validation and intentionally ignored record types, and remove a
redundant PAGE_SIZE check.
- Validate the CFR payload checksum before parsing it.
- Make EFI writes transactional: lock the complete operation, restore
both the variable and live state on failure, and preserve a pending
reboot indication when rollback cannot fully restore the setting.
- Lock EFI reads and expose settings read-only when EFI writes are not
supported.
- Skip settings beneath suppressed or dependency-gated forms, propagate
enclosing read-only/inactive state, and bound CFR form nesting.
- Propagate transient probe failures instead of silently dropping
settings.
- Preserve APM token zero and skip an enumeration if its values cannot
fit in the sysfs output buffer.
- Simplify the device_create() error path.
Testing:
- x86_64 allmodconfig W=1 builds of the CFR driver, firmware attributes
helper, and all converted platform/x86 consumers.
- UML allmodconfig W=1 builds of the CFR driver and helper, exercising
the path compiled without CONFIG_X86.
Sean Rhodes (3):
firmware: Move firmware attributes class helper
firmware: Rename google firmware directory to coreboot
firmware: coreboot: Add CFR firmware attributes driver
MAINTAINERS | 18 +-
drivers/firmware/Kconfig | 5 +-
drivers/firmware/Makefile | 3 +-
drivers/firmware/{google => coreboot}/Kconfig | 20 +-
.../firmware/{google => coreboot}/Makefile | 1 +
drivers/firmware/{google => coreboot}/cbmem.c | 0
drivers/firmware/coreboot/coreboot-cfr.c | 1141 +++++++++++++++++
.../{google => coreboot}/coreboot_table.c | 0
.../{google => coreboot}/coreboot_table.h | 0
.../framebuffer-coreboot.c | 0
drivers/firmware/{google => coreboot}/gsmi.c | 0
.../memconsole-coreboot.c | 0
.../memconsole-x86-legacy.c | 0
.../{google => coreboot}/memconsole.c | 0
.../{google => coreboot}/memconsole.h | 6 +-
drivers/firmware/{google => coreboot}/vpd.c | 0
.../{google => coreboot}/vpd_decode.c | 0
.../{google => coreboot}/vpd_decode.h | 0
.../firmware_attributes_class.c | 2 +-
drivers/platform/x86/Kconfig | 3 -
drivers/platform/x86/Makefile | 2 -
drivers/platform/x86/asus-armoury.c | 2 +-
.../x86/dell/dell-wmi-sysman/sysman.c | 2 +-
drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 2 +-
drivers/platform/x86/lenovo/think-lmi.c | 2 +-
drivers/platform/x86/lenovo/wmi-other.c | 2 +-
drivers/platform/x86/samsung-galaxybook.c | 2 +-
.../linux/firmware_attributes.h | 6 +-
28 files changed, 1194 insertions(+), 25 deletions(-)
rename drivers/firmware/{google => coreboot}/Kconfig (82%)
rename drivers/firmware/{google => coreboot}/Makefile (92%)
rename drivers/firmware/{google => coreboot}/cbmem.c (100%)
create mode 100644 drivers/firmware/coreboot/coreboot-cfr.c
rename drivers/firmware/{google => coreboot}/coreboot_table.c (100%)
rename drivers/firmware/{google => coreboot}/coreboot_table.h (100%)
rename drivers/firmware/{google => coreboot}/framebuffer-coreboot.c (100%)
rename drivers/firmware/{google => coreboot}/gsmi.c (100%)
rename drivers/firmware/{google => coreboot}/memconsole-coreboot.c (100%)
rename drivers/firmware/{google => coreboot}/memconsole-x86-legacy.c (100%)
rename drivers/firmware/{google => coreboot}/memconsole.c (100%)
rename drivers/firmware/{google => coreboot}/memconsole.h (82%)
rename drivers/firmware/{google => coreboot}/vpd.c (100%)
rename drivers/firmware/{google => coreboot}/vpd_decode.c (100%)
rename drivers/firmware/{google => coreboot}/vpd_decode.h (100%)
rename drivers/{platform/x86 => firmware}/firmware_attributes_class.c (94%)
rename drivers/platform/x86/firmware_attributes_class.h => include/linux/firmware_attributes.h (60%)
Range-diff against v6:
1: 02edc6d05059 ! 1: c414cb12d7ac firmware: Move firmware attributes class helper
@@ Commit message
Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com>
+ Tested-by: Oliver Lin <oliver@liuxiaozhen.dev>
Signed-off-by: Sean Rhodes <sean@starlabs.systems>
## MAINTAINERS ##
@@ drivers/platform/x86/Makefile: obj-$(CONFIG_X86_PLATFORM_DRIVERS_HP) += hp/
obj-$(CONFIG_SENSORS_HDAPS) += hdaps.o
## drivers/platform/x86/asus-armoury.c ##
-@@
- #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>
@@
#include <linux/sysfs.h>
#include "asus-armoury.h"
-#include "firmware_attributes_class.h"
++#include <linux/firmware_attributes.h>
#define ASUS_NB_WMI_EVENT_GUID "0B3CBB35-E3C2-45ED-91C2-4C5A6D195D1C"
## drivers/platform/x86/dell/dell-wmi-sysman/sysman.c ##
@@
- #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"
++#include <linux/firmware_attributes.h>
#define MAX_TYPES 4
--#include <linux/nls.h>
-
- struct wmi_sysman_priv wmi_priv = {
- .mutex = __MUTEX_INITIALIZER(wmi_priv.mutex),
+ #include <linux/nls.h>
## drivers/platform/x86/hp/hp-bioscfg/bioscfg.c ##
@@
-
- #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>
++#include <linux/firmware_attributes.h>
+ #include <linux/nls.h>
+ #include <linux/errno.h>
- MODULE_AUTHOR("Jorge Lopez <jorge.lopez2@hp.com>");
- MODULE_DESCRIPTION("HP BIOS Configuration Driver");
## drivers/platform/x86/lenovo/think-lmi.c ##
@@
-
- #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/dmi.h>
#include <linux/wmi.h>
-#include "../firmware_attributes_class.h"
-+
++#include <linux/firmware_attributes.h>
#include "think-lmi.h"
static bool debug_support;
## drivers/platform/x86/lenovo/wmi-other.c ##
-@@
- #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>
@@
#include "wmi-capdata.h"
#include "wmi-events.h"
#include "wmi-helpers.h"
-#include "../firmware_attributes_class.h"
++#include <linux/firmware_attributes.h>
#define LENOVO_OTHER_MODE_GUID "DC2A8805-3A8C-41BA-A6F7-092E0089CD3B"
## drivers/platform/x86/samsung-galaxybook.c ##
@@
- #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>
-@@
- #include <linux/sysfs.h>
#include <linux/uuid.h>
#include <linux/workqueue.h>
-+
#include <acpi/battery.h>
-#include "firmware_attributes_class.h"
++#include <linux/firmware_attributes.h>
#define DRIVER_NAME "samsung-galaxybook"
2: d5d3d1334994 = 2: 198e2d4dff44 firmware: Rename google firmware directory to coreboot
3: 7a65c6b0e631 ! 3: f4842df09f6e firmware: coreboot: Add CFR firmware attributes driver
@@ drivers/firmware/coreboot/Kconfig: config GOOGLE_COREBOOT_TABLE
+ tristate "coreboot CFR firmware attributes"
+ depends on EFI
+ depends on GOOGLE_COREBOOT_TABLE
++ select CRC32
+ select FW_ATTR_CLASS
+ help
+ This driver parses the coreboot CFR table and exposes runtime EFI
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+#include <linux/array_size.h>
+#include <linux/bitops.h>
+#include <linux/cleanup.h>
++#include <linux/crc32.h>
+#include <linux/ctype.h>
+#include <linux/device.h>
+#include <linux/device-id/coreboot.h>
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+#define COREBOOT_CFR_APM_CNT_PORT 0xb2
+#define COREBOOT_CFR_APM_STS_PORT 0xb3
+#define COREBOOT_CFR_APM_APPLY_CMD 0xe3
++#define COREBOOT_CFR_MAX_FORM_DEPTH 16
+
+static efi_guid_t coreboot_cfr_guid = EFI_GUID(0xceae4c1d, 0x335b, 0x4685,
+ 0xa4, 0xa0, 0xfc, 0x4a,
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ struct list_head settings;
+ /* Serializes EFI variable writes and the matching runtime apply hook. */
+ struct mutex lock;
++ bool efi_writes_supported;
+ bool pending_reboot;
+};
+
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+coreboot_cfr_child_entry(const void *base, size_t len, u32 tag)
+{
+ const struct coreboot_table_entry *entry;
-+ size_t off = 0;
++ size_t off;
+
-+ while (off < len) {
++ for (off = 0; off < len; off += entry->size) {
+ if (len - off < sizeof(*entry))
+ return NULL;
+
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+
+ if (entry->tag == tag)
+ return entry;
-+
-+ off += entry->size;
+ }
+
+ return NULL;
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ return no_free_ptr(efi_name);
+}
+
-+static int coreboot_cfr_read_value(const struct coreboot_cfr_setting *setting,
-+ u32 *value, u32 *attrs)
++static int coreboot_cfr_read_efi_value_locked(efi_char16_t *efi_name,
++ u32 *value, u32 *attrs)
+{
+ unsigned long size = sizeof(__le32);
-+ efi_char16_t *efi_name;
+ efi_status_t status;
+ __le32 data;
+ u32 attr;
+
-+ efi_name = coreboot_cfr_efi_name(setting->name);
-+ if (IS_ERR(efi_name))
-+ return PTR_ERR(efi_name);
-+
+ status = efivar_get_variable(efi_name, &coreboot_cfr_guid, &attr,
+ &size, &data);
-+ kfree(efi_name);
+ if (status != EFI_SUCCESS)
+ return efi_status_to_err(status);
+
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ return 0;
+}
+
-+static int coreboot_cfr_write_efi_value(struct coreboot_cfr_setting *setting,
-+ u32 value, u32 attrs)
++static int coreboot_cfr_read_value(const struct coreboot_cfr_setting *setting,
++ u32 *value, u32 *attrs)
+{
+ efi_char16_t *efi_name;
-+ efi_status_t status;
-+ __le32 data;
++ int ret;
+
+ efi_name = coreboot_cfr_efi_name(setting->name);
+ if (IS_ERR(efi_name))
+ return PTR_ERR(efi_name);
+
-+ data = cpu_to_le32(value);
-+ status = efivar_set_variable(efi_name, &coreboot_cfr_guid, attrs,
-+ sizeof(data), &data);
++ ret = efivar_lock();
++ if (ret) {
++ kfree(efi_name);
++ return ret;
++ }
++
++ ret = coreboot_cfr_read_efi_value_locked(efi_name, value, attrs);
++ efivar_unlock();
+ kfree(efi_name);
++
++ return ret;
++}
++
++static int coreboot_cfr_write_efi_value_locked(efi_char16_t *efi_name,
++ u32 value, u32 attrs)
++{
++ efi_status_t status;
++ __le32 data;
++
++ if (!efivar_supports_writes())
++ return -EROFS;
++
++ data = cpu_to_le32(value);
++ status = efivar_set_variable_locked(efi_name, &coreboot_cfr_guid, attrs,
++ sizeof(data), &data, false);
+ if (status != EFI_SUCCESS)
+ return efi_status_to_err(status);
+
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+#ifdef CONFIG_X86
+ u8 status;
+
-+ if (setting->runtime_apply_method != CFR_RUNTIME_APPLY_APM_CNT ||
-+ !setting->runtime_apply_id)
++ if (setting->runtime_apply_method != CFR_RUNTIME_APPLY_APM_CNT)
+ return -EOPNOTSUPP;
+
+ outb((u8)setting->runtime_apply_id, COREBOOT_CFR_APM_STS_PORT);
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+static int coreboot_cfr_write_value(struct coreboot_cfr_setting *setting,
+ u32 value)
+{
++ efi_char16_t *efi_name;
+ u32 attrs;
+ u32 old;
++ int restore_ret;
+ int ret;
+ bool changed = false;
+
+ if (setting->read_only)
+ return -EACCES;
+
-+ scoped_guard(mutex, &setting->drvdata->lock) {
-+ ret = coreboot_cfr_read_value(setting, &old, &attrs);
-+ if (ret)
-+ return ret;
++ efi_name = coreboot_cfr_efi_name(setting->name);
++ if (IS_ERR(efi_name))
++ return PTR_ERR(efi_name);
+
-+ if ((attrs & COREBOOT_CFR_EFI_ATTRS) != COREBOOT_CFR_EFI_ATTRS)
-+ return -EOPNOTSUPP;
++ mutex_lock(&setting->drvdata->lock);
++ ret = efivar_lock();
++ if (ret)
++ goto out_unlock_mutex;
++
++ ret = coreboot_cfr_read_efi_value_locked(efi_name, &old, &attrs);
++ if (ret)
++ goto out_unlock_efi;
+
-+ if (old == value)
-+ return 0;
++ if ((attrs & COREBOOT_CFR_EFI_ATTRS) != COREBOOT_CFR_EFI_ATTRS) {
++ ret = -EOPNOTSUPP;
++ goto out_unlock_efi;
++ }
+
-+ ret = coreboot_cfr_write_efi_value(setting, value, attrs);
-+ if (ret)
-+ return ret;
-+ changed = true;
++ if (old == value)
++ goto out_unlock_efi;
+
-+ ret = coreboot_cfr_apply_runtime(setting);
-+ if (ret == -EOPNOTSUPP) {
-+ /* EFI changed; firmware will consume it after reboot. */
++ ret = coreboot_cfr_write_efi_value_locked(efi_name, value, attrs);
++ if (ret)
++ goto out_unlock_efi;
++ changed = true;
++
++ ret = coreboot_cfr_apply_runtime(setting);
++ if (ret == -EOPNOTSUPP) {
++ /* EFI changed; firmware will consume it after reboot. */
++ setting->drvdata->pending_reboot = true;
++ ret = 0;
++ } else if (ret) {
++ restore_ret = coreboot_cfr_write_efi_value_locked(efi_name, old, attrs);
++ if (restore_ret) {
+ setting->drvdata->pending_reboot = true;
-+ ret = 0;
-+ } else if (ret) {
-+ int restore_ret;
-+
-+ restore_ret = coreboot_cfr_write_efi_value(setting, old, attrs);
-+ if (restore_ret)
-+ ret = restore_ret;
-+ else
-+ changed = false;
++ ret = restore_ret;
++ } else if (coreboot_cfr_apply_runtime(setting)) {
++ setting->drvdata->pending_reboot = true;
++ } else {
++ changed = false;
+ }
+ }
+
++out_unlock_efi:
++ efivar_unlock();
++out_unlock_mutex:
++ mutex_unlock(&setting->drvdata->lock);
++ kfree(efi_name);
++
+ if (changed)
+ kobject_uevent(&setting->drvdata->class_dev->kobj, KOBJ_CHANGE);
+ return ret;
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+{
+ u32 delta;
+
-+ if (setting->type != COREBOOT_CFR_SETTING_NUMBER)
++ if (setting->type != COREBOOT_CFR_SETTING_NUMBER) {
++ /* Enum and bool values are valid only if they have a label. */
+ return coreboot_cfr_label_from_value(setting, value);
++ }
+
+ if (value < setting->min || value > setting->max)
+ return false;
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ for (i = 0; i < setting->n_values; i++) {
+ len += sysfs_emit_at(buf, len, "%s%s", i ? ";" : "",
+ setting->values[i].label);
-+ if (len >= PAGE_SIZE)
-+ return len;
+ }
+
+ len += sysfs_emit_at(buf, len, "\n");
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+static int coreboot_cfr_count_enum_values(const void *base, size_t len)
+{
+ const struct coreboot_table_entry *entry;
-+ size_t off = 0;
++ size_t off;
+ int count = 0;
+
-+ while (off < len) {
++ for (off = 0; off < len; off += entry->size) {
+ if (len - off < sizeof(*entry))
+ return -EINVAL;
+
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+
+ if (entry->tag == CFR_TAG_ENUM_VALUE)
+ count++;
-+
-+ off += entry->size;
+ }
+
+ return count;
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ const struct lb_cfr_enum_value *enum_value;
+ const struct lb_cfr_varbinary *label;
+ const struct coreboot_table_entry *entry;
-+ size_t off = 0;
++ size_t off;
+ int count;
+
+ count = coreboot_cfr_count_enum_values(base, len);
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ if (!setting->values)
+ return -ENOMEM;
+
-+ while (off < len) {
++ for (off = 0; off < len; off += entry->size) {
+ entry = base + off;
-+ if (entry->tag != CFR_TAG_ENUM_VALUE) {
-+ off += entry->size;
++ if (entry->tag != CFR_TAG_ENUM_VALUE)
+ continue;
-+ }
+
+ if (entry->size < sizeof(*enum_value))
+ return -EINVAL;
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+
+ setting->values[setting->n_values].value = enum_value->value;
+ setting->n_values++;
-+ off += entry->size;
+ }
+
+ return 0;
+}
+
++static bool
++coreboot_cfr_possible_values_fit(const struct coreboot_cfr_setting *setting)
++{
++ size_t len = 1; /* Trailing newline. */
++ unsigned int i;
++
++ for (i = 0; i < setting->n_values; i++) {
++ if (i)
++ len++;
++
++ if (strlen(setting->values[i].label) >= PAGE_SIZE - len)
++ return false;
++
++ len += strlen(setting->values[i].label);
++ }
++
++ return true;
++}
++
+static int coreboot_cfr_setting_is_usable(struct coreboot_cfr_setting *setting)
+{
+ u32 value;
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+}
+
+static int coreboot_cfr_add_numeric_option(struct coreboot_cfr_drvdata *data,
-+ const struct lb_cfr_numeric_option *option)
++ const struct lb_cfr_numeric_option *option,
++ bool parent_read_only)
+{
+ const struct lb_cfr_varbinary *name;
+ const struct lb_cfr_varbinary *display_name;
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ setting->min = option->min;
+ setting->max = option->max;
+ setting->step = option->step ?: 1;
-+ setting->read_only = option->flags & COREBOOT_CFR_OPT_READ_ONLY_FLAGS;
++ setting->read_only =
++ (option->flags & COREBOOT_CFR_OPT_READ_ONLY_FLAGS) ||
++ !data->efi_writes_supported || parent_read_only;
+
+ runtime_apply = coreboot_cfr_child_runtime_apply(child_base, child_len);
+ if (IS_ERR(runtime_apply)) {
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ goto err_put_setting;
+ }
+
-+ if (runtime_apply && runtime_apply->method == CFR_RUNTIME_APPLY_APM_CNT &&
-+ runtime_apply->id) {
++ if (runtime_apply && runtime_apply->method == CFR_RUNTIME_APPLY_APM_CNT) {
+ setting->runtime_apply_method = runtime_apply->method;
+ setting->runtime_apply_id = runtime_apply->id;
+ }
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+
+ if (ret)
+ goto err_put_setting;
++ if (setting->type != COREBOOT_CFR_SETTING_NUMBER &&
++ !coreboot_cfr_possible_values_fit(setting)) {
++ ret = 0;
++ goto err_put_setting;
++ }
+
+ ret = coreboot_cfr_setting_is_usable(setting);
+ if (ret) {
-+ ret = 0;
++ if (ret == -ENOENT || ret == -EINVAL || ret == -EOPNOTSUPP ||
++ ret == -ENAMETOOLONG)
++ ret = 0;
+ goto err_put_setting;
+ }
+
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+}
+
+static int coreboot_cfr_parse_records(struct coreboot_cfr_drvdata *data,
-+ const void *base, size_t len)
++ const void *base, size_t len,
++ unsigned int depth,
++ bool parent_read_only)
+{
++ const struct lb_cfr_option_form *form;
+ const struct coreboot_table_entry *entry;
+ const void *child_base;
+ size_t child_len;
-+ size_t off = 0;
++ size_t off;
+ int ret;
+
-+ while (off < len) {
++ for (off = 0; off < len; off += entry->size) {
+ if (len - off < sizeof(*entry))
+ return -EINVAL;
+
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ if (entry->size < sizeof(struct lb_cfr_option_form))
+ return -EINVAL;
+
++ form = base + off;
++ if (form->dependency_id ||
++ form->flags & CFR_OPTFLAG_SUPPRESS)
++ break;
++
++ if (depth >= COREBOOT_CFR_MAX_FORM_DEPTH)
++ return -E2BIG;
++
+ child_base = base + off + sizeof(struct lb_cfr_option_form);
+ child_len = entry->size - sizeof(struct lb_cfr_option_form);
+ ret = coreboot_cfr_parse_records(data, child_base,
-+ child_len);
++ child_len, depth + 1,
++ parent_read_only ||
++ (form->flags &
++ COREBOOT_CFR_OPT_READ_ONLY_FLAGS));
+ if (ret)
+ return ret;
+ break;
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ case CFR_TAG_OPTION_BOOL:
+ if (entry->size < sizeof(struct lb_cfr_numeric_option))
+ return -EINVAL;
-+ ret = coreboot_cfr_add_numeric_option(data, base + off);
++ ret = coreboot_cfr_add_numeric_option(data, base + off,
++ parent_read_only);
+ if (ret)
+ return ret;
+ break;
+ default:
++ /* Ignore unsupported, child-only and future record types. */
+ break;
+ }
-+
-+ off += entry->size;
+ }
+
+ return 0;
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ if (root->size < sizeof(*root) || root->size > dev->entry.size)
+ return -EINVAL;
+
++ payload_len = root->size - sizeof(*root);
++ if (crc32_be(0, root + 1, payload_len) != root->checksum)
++ return -EBADMSG;
++
+ if (!efivar_is_available())
+ return -EPROBE_DEFER;
+
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ if (!data)
+ return -ENOMEM;
+
++ ret = efivar_lock();
++ if (ret)
++ return ret;
++ data->efi_writes_supported = efivar_supports_writes();
++ efivar_unlock();
++
+ INIT_LIST_HEAD(&data->settings);
+ ret = devm_mutex_init(&dev->dev, &data->lock);
+ if (ret)
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+
+ data->class_dev = device_create(&firmware_attributes_class, NULL,
+ MKDEV(0, 0), NULL, DRIVER_NAME);
-+ if (IS_ERR(data->class_dev)) {
-+ ret = PTR_ERR(data->class_dev);
-+ goto err_clear_data;
-+ }
++ if (IS_ERR(data->class_dev))
++ return PTR_ERR(data->class_dev);
+ dev_set_drvdata(data->class_dev, data);
+
+ data->attrs_kset = kset_create_and_add("attributes", NULL,
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ if (ret)
+ goto err_unregister_attrs;
+
-+ payload_len = root->size - sizeof(*root);
-+ ret = coreboot_cfr_parse_records(data, root + 1, payload_len);
++ ret = coreboot_cfr_parse_records(data, root + 1, payload_len, 0, false);
+ if (ret)
+ goto err_unregister_settings;
+
@@ drivers/firmware/coreboot/coreboot-cfr.c (new)
+ kset_unregister(data->attrs_kset);
+err_unregister_dev:
+ device_unregister(data->class_dev);
-+err_clear_data:
+ return ret;
+}
+
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v7 1/3] firmware: Move firmware attributes class helper
2026-07-17 8:50 [PATCH v7 0/3] firmware: Add coreboot CFR firmware attributes driver Sean Rhodes
@ 2026-07-17 8:50 ` Sean Rhodes
2026-07-18 2:22 ` Derek J. Clark
2026-07-17 8:50 ` [PATCH v7 2/3] firmware: Rename google firmware directory to coreboot Sean Rhodes
2026-07-17 8:50 ` [PATCH v7 3/3] firmware: coreboot: Add CFR firmware attributes driver Sean Rhodes
2 siblings, 1 reply; 6+ messages in thread
From: Sean Rhodes @ 2026-07-17 8:50 UTC (permalink / raw)
To: tzungbi
Cc: hansg, ilpo.jarvinen, corentin.chary, luke, denis.benato,
prasanth.ksr, jorge.lopez2, mpearson-lenovo, derekjohn.clark,
josh, briannorris, jwerner, tzimmermann, javierm, kees,
u.kleine-koenig, mst, chenhuacai, wenst, florian.fainelli,
titouan.ameline, oliver, linux-kernel, platform-driver-x86,
Dell.Client.Kernel, chrome-platform, Sean Rhodes
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>
Tested-by: Oliver Lin <oliver@liuxiaozhen.dev>
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 | 2 +-
drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 2 +-
drivers/platform/x86/lenovo/think-lmi.c | 2 +-
drivers/platform/x86/lenovo/wmi-other.c | 2 +-
drivers/platform/x86/samsung-galaxybook.c | 2 +-
.../linux/firmware_attributes.h | 6 +++---
13 files changed, 22 insertions(+), 15 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..93fac0d464d2 100644
--- a/drivers/platform/x86/asus-armoury.c
+++ b/drivers/platform/x86/asus-armoury.c
@@ -34,7 +34,7 @@
#include <linux/sysfs.h>
#include "asus-armoury.h"
-#include "firmware_attributes_class.h"
+#include <linux/firmware_attributes.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..85670be01647 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
@@ -16,7 +16,7 @@
#include <linux/sysfs.h>
#include <linux/wmi.h>
#include "dell-wmi-sysman.h"
-#include "../../firmware_attributes_class.h"
+#include <linux/firmware_attributes.h>
#define MAX_TYPES 4
#include <linux/nls.h>
diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index 27fd6cd21529..44314ae4e5e3 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -14,7 +14,7 @@
#include <linux/string.h>
#include <linux/wmi.h>
#include "bioscfg.h"
-#include "../../firmware_attributes_class.h"
+#include <linux/firmware_attributes.h>
#include <linux/nls.h>
#include <linux/errno.h>
diff --git a/drivers/platform/x86/lenovo/think-lmi.c b/drivers/platform/x86/lenovo/think-lmi.c
index e215e86e3db7..a56f152b5f28 100644
--- a/drivers/platform/x86/lenovo/think-lmi.c
+++ b/drivers/platform/x86/lenovo/think-lmi.c
@@ -20,7 +20,7 @@
#include <linux/types.h>
#include <linux/dmi.h>
#include <linux/wmi.h>
-#include "../firmware_attributes_class.h"
+#include <linux/firmware_attributes.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..e9a2c4684e5f 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -50,7 +50,7 @@
#include "wmi-capdata.h"
#include "wmi-events.h"
#include "wmi-helpers.h"
-#include "../firmware_attributes_class.h"
+#include <linux/firmware_attributes.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..74b8c887dd79 100644
--- a/drivers/platform/x86/samsung-galaxybook.c
+++ b/drivers/platform/x86/samsung-galaxybook.c
@@ -28,7 +28,7 @@
#include <linux/uuid.h>
#include <linux/workqueue.h>
#include <acpi/battery.h>
-#include "firmware_attributes_class.h"
+#include <linux/firmware_attributes.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 */
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v7 2/3] firmware: Rename google firmware directory to coreboot
2026-07-17 8:50 [PATCH v7 0/3] firmware: Add coreboot CFR firmware attributes driver Sean Rhodes
2026-07-17 8:50 ` [PATCH v7 1/3] firmware: Move firmware attributes class helper Sean Rhodes
@ 2026-07-17 8:50 ` Sean Rhodes
2026-07-18 0:39 ` Julius Werner
2026-07-17 8:50 ` [PATCH v7 3/3] firmware: coreboot: Add CFR firmware attributes driver Sean Rhodes
2 siblings, 1 reply; 6+ messages in thread
From: Sean Rhodes @ 2026-07-17 8:50 UTC (permalink / raw)
To: tzungbi
Cc: hansg, ilpo.jarvinen, corentin.chary, luke, denis.benato,
prasanth.ksr, jorge.lopez2, mpearson-lenovo, derekjohn.clark,
josh, briannorris, jwerner, tzimmermann, javierm, kees,
u.kleine-koenig, mst, chenhuacai, wenst, florian.fainelli,
titouan.ameline, oliver, linux-kernel, platform-driver-x86,
Dell.Client.Kernel, chrome-platform, Sean Rhodes
Most drivers in drivers/firmware/google are coreboot table drivers rather
than Google-specific firmware drivers. Rename the directory to coreboot
and update the Kconfig prompt, build path and maintainer entry.
Keep the existing CONFIG_GOOGLE_* symbols to avoid user configuration
churn in this mechanical move.
Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
MAINTAINERS | 4 ++--
drivers/firmware/Kconfig | 2 +-
drivers/firmware/Makefile | 2 +-
drivers/firmware/{google => coreboot}/Kconfig | 6 +++---
drivers/firmware/{google => coreboot}/Makefile | 0
drivers/firmware/{google => coreboot}/cbmem.c | 0
drivers/firmware/{google => coreboot}/coreboot_table.c | 0
drivers/firmware/{google => coreboot}/coreboot_table.h | 0
.../firmware/{google => coreboot}/framebuffer-coreboot.c | 0
drivers/firmware/{google => coreboot}/gsmi.c | 0
drivers/firmware/{google => coreboot}/memconsole-coreboot.c | 0
.../firmware/{google => coreboot}/memconsole-x86-legacy.c | 0
drivers/firmware/{google => coreboot}/memconsole.c | 0
drivers/firmware/{google => coreboot}/memconsole.h | 6 +++---
drivers/firmware/{google => coreboot}/vpd.c | 0
drivers/firmware/{google => coreboot}/vpd_decode.c | 0
drivers/firmware/{google => coreboot}/vpd_decode.h | 0
17 files changed, 10 insertions(+), 10 deletions(-)
rename drivers/firmware/{google => coreboot}/Kconfig (94%)
rename drivers/firmware/{google => coreboot}/Makefile (100%)
rename drivers/firmware/{google => coreboot}/cbmem.c (100%)
rename drivers/firmware/{google => coreboot}/coreboot_table.c (100%)
rename drivers/firmware/{google => coreboot}/coreboot_table.h (100%)
rename drivers/firmware/{google => coreboot}/framebuffer-coreboot.c (100%)
rename drivers/firmware/{google => coreboot}/gsmi.c (100%)
rename drivers/firmware/{google => coreboot}/memconsole-coreboot.c (100%)
rename drivers/firmware/{google => coreboot}/memconsole-x86-legacy.c (100%)
rename drivers/firmware/{google => coreboot}/memconsole.c (100%)
rename drivers/firmware/{google => coreboot}/memconsole.h (82%)
rename drivers/firmware/{google => coreboot}/vpd.c (100%)
rename drivers/firmware/{google => coreboot}/vpd_decode.c (100%)
rename drivers/firmware/{google => coreboot}/vpd_decode.h (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 07bdba98b1b1..8cf2766d515d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11068,14 +11068,14 @@ S: Maintained
F: Documentation/networking/device_drivers/ethernet/google/gve.rst
F: drivers/net/ethernet/google
-GOOGLE FIRMWARE DRIVERS
+COREBOOT FIRMWARE DRIVERS
M: Tzung-Bi Shih <tzungbi@kernel.org>
R: Brian Norris <briannorris@chromium.org>
R: Julius Werner <jwerner@chromium.org>
L: chrome-platform@lists.linux.dev
S: Maintained
T: git git://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git
-F: drivers/firmware/google/
+F: drivers/firmware/coreboot/
F: include/linux/coreboot.h
GOOGLE TENSOR SoC SUPPORT
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 78a239f8bbef..4df0a0ad18b8 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -293,7 +293,7 @@ endif # TURRIS_MOX_RWTM
source "drivers/firmware/arm_ffa/Kconfig"
source "drivers/firmware/broadcom/Kconfig"
source "drivers/firmware/cirrus/Kconfig"
-source "drivers/firmware/google/Kconfig"
+source "drivers/firmware/coreboot/Kconfig"
source "drivers/firmware/efi/Kconfig"
source "drivers/firmware/imx/Kconfig"
source "drivers/firmware/meson/Kconfig"
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 385b6e8b7fc6..7a8fc3eeb3f0 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -30,7 +30,7 @@ obj-y += broadcom/
obj-y += cirrus/
obj-y += meson/
obj-y += microchip/
-obj-$(CONFIG_GOOGLE_FIRMWARE) += google/
+obj-$(CONFIG_GOOGLE_FIRMWARE) += coreboot/
obj-y += efi/
obj-y += imx/
obj-y += psci/
diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/coreboot/Kconfig
similarity index 94%
rename from drivers/firmware/google/Kconfig
rename to drivers/firmware/coreboot/Kconfig
index b78c644fa253..b248bafe8b6b 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/coreboot/Kconfig
@@ -1,10 +1,10 @@
# SPDX-License-Identifier: GPL-2.0-only
menuconfig GOOGLE_FIRMWARE
- bool "Google Firmware Drivers"
+ bool "coreboot firmware drivers"
default n
help
- These firmware drivers are used by Google servers,
- Chromebooks and other devices using coreboot firmware.
+ These firmware drivers are used by devices running coreboot
+ firmware, including Chromebooks and some Google servers.
If in doubt, say "N".
if GOOGLE_FIRMWARE
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/coreboot/Makefile
similarity index 100%
rename from drivers/firmware/google/Makefile
rename to drivers/firmware/coreboot/Makefile
diff --git a/drivers/firmware/google/cbmem.c b/drivers/firmware/coreboot/cbmem.c
similarity index 100%
rename from drivers/firmware/google/cbmem.c
rename to drivers/firmware/coreboot/cbmem.c
diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/coreboot/coreboot_table.c
similarity index 100%
rename from drivers/firmware/google/coreboot_table.c
rename to drivers/firmware/coreboot/coreboot_table.c
diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/coreboot/coreboot_table.h
similarity index 100%
rename from drivers/firmware/google/coreboot_table.h
rename to drivers/firmware/coreboot/coreboot_table.h
diff --git a/drivers/firmware/google/framebuffer-coreboot.c b/drivers/firmware/coreboot/framebuffer-coreboot.c
similarity index 100%
rename from drivers/firmware/google/framebuffer-coreboot.c
rename to drivers/firmware/coreboot/framebuffer-coreboot.c
diff --git a/drivers/firmware/google/gsmi.c b/drivers/firmware/coreboot/gsmi.c
similarity index 100%
rename from drivers/firmware/google/gsmi.c
rename to drivers/firmware/coreboot/gsmi.c
diff --git a/drivers/firmware/google/memconsole-coreboot.c b/drivers/firmware/coreboot/memconsole-coreboot.c
similarity index 100%
rename from drivers/firmware/google/memconsole-coreboot.c
rename to drivers/firmware/coreboot/memconsole-coreboot.c
diff --git a/drivers/firmware/google/memconsole-x86-legacy.c b/drivers/firmware/coreboot/memconsole-x86-legacy.c
similarity index 100%
rename from drivers/firmware/google/memconsole-x86-legacy.c
rename to drivers/firmware/coreboot/memconsole-x86-legacy.c
diff --git a/drivers/firmware/google/memconsole.c b/drivers/firmware/coreboot/memconsole.c
similarity index 100%
rename from drivers/firmware/google/memconsole.c
rename to drivers/firmware/coreboot/memconsole.c
diff --git a/drivers/firmware/google/memconsole.h b/drivers/firmware/coreboot/memconsole.h
similarity index 82%
rename from drivers/firmware/google/memconsole.h
rename to drivers/firmware/coreboot/memconsole.h
index aaff2b72b606..58899c2abaaa 100644
--- a/drivers/firmware/google/memconsole.h
+++ b/drivers/firmware/coreboot/memconsole.h
@@ -7,8 +7,8 @@
* Copyright 2017 Google Inc.
*/
-#ifndef __FIRMWARE_GOOGLE_MEMCONSOLE_H
-#define __FIRMWARE_GOOGLE_MEMCONSOLE_H
+#ifndef __FIRMWARE_COREBOOT_MEMCONSOLE_H
+#define __FIRMWARE_COREBOOT_MEMCONSOLE_H
#include <linux/types.h>
@@ -33,4 +33,4 @@ int memconsole_sysfs_init(void);
*/
void memconsole_exit(void);
-#endif /* __FIRMWARE_GOOGLE_MEMCONSOLE_H */
+#endif /* __FIRMWARE_COREBOOT_MEMCONSOLE_H */
diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/coreboot/vpd.c
similarity index 100%
rename from drivers/firmware/google/vpd.c
rename to drivers/firmware/coreboot/vpd.c
diff --git a/drivers/firmware/google/vpd_decode.c b/drivers/firmware/coreboot/vpd_decode.c
similarity index 100%
rename from drivers/firmware/google/vpd_decode.c
rename to drivers/firmware/coreboot/vpd_decode.c
diff --git a/drivers/firmware/google/vpd_decode.h b/drivers/firmware/coreboot/vpd_decode.h
similarity index 100%
rename from drivers/firmware/google/vpd_decode.h
rename to drivers/firmware/coreboot/vpd_decode.h
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v7 3/3] firmware: coreboot: Add CFR firmware attributes driver
2026-07-17 8:50 [PATCH v7 0/3] firmware: Add coreboot CFR firmware attributes driver Sean Rhodes
2026-07-17 8:50 ` [PATCH v7 1/3] firmware: Move firmware attributes class helper Sean Rhodes
2026-07-17 8:50 ` [PATCH v7 2/3] firmware: Rename google firmware directory to coreboot Sean Rhodes
@ 2026-07-17 8:50 ` Sean Rhodes
2 siblings, 0 replies; 6+ messages in thread
From: Sean Rhodes @ 2026-07-17 8:50 UTC (permalink / raw)
To: tzungbi
Cc: hansg, ilpo.jarvinen, corentin.chary, luke, denis.benato,
prasanth.ksr, jorge.lopez2, mpearson-lenovo, derekjohn.clark,
josh, briannorris, jwerner, tzimmermann, javierm, kees,
u.kleine-koenig, mst, chenhuacai, wenst, florian.fainelli,
titouan.ameline, oliver, linux-kernel, platform-driver-x86,
Dell.Client.Kernel, chrome-platform, Sean Rhodes
coreboot can publish a CFR tree in its coreboot table to describe
firmware setup options. Add a firmware-attributes driver for that table
entry under the coreboot firmware driver directory.
The driver exposes runtime EFI variable-backed boolean, enum and integer
settings through the firmware-attributes ABI. It only registers settings
with readable runtime EFI variables, skips suppressed, volatile,
dependency-gated and non-runtime options, and keeps read-only or inactive
options read-only in sysfs.
For settings with an APM_CNT runtime-apply record, write the EFI variable
and ask firmware to apply the setting immediately on x86. If firmware
reports a failure, restore the old EFI variable value. Settings without a
runtime apply hook, or non-x86 builds, keep the EFI variable update and
set pending_reboot.
Tested on a Star Labs Byte with firmware 26.07 and the driver built as a
module against Linux 6.17: runtime APM_CNT apply was verified with
power_led, and reboot-needed behavior was verified with debug_level.
Link: https://doc.coreboot.org/drivers/cfr.html
Link: https://doc.coreboot.org/drivers/cfr_internal.html
Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
MAINTAINERS | 6 +
drivers/firmware/coreboot/Kconfig | 14 +
drivers/firmware/coreboot/Makefile | 1 +
drivers/firmware/coreboot/coreboot-cfr.c | 1141 ++++++++++++++++++++++
4 files changed, 1162 insertions(+)
create mode 100644 drivers/firmware/coreboot/coreboot-cfr.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 8cf2766d515d..a00f54e0324e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6665,6 +6665,12 @@ F: tools/testing/selftests/cgroup/test_hugetlb_memcg.c
F: tools/testing/selftests/cgroup/test_kmem.c
F: tools/testing/selftests/cgroup/test_memcontrol.c
+COREBOOT CFR FIRMWARE ATTRIBUTES DRIVER
+M: Sean Rhodes <sean@starlabs.systems>
+L: chrome-platform@lists.linux.dev
+S: Maintained
+F: drivers/firmware/coreboot/coreboot-cfr.c
+
CORETEMP HARDWARE MONITORING DRIVER
L: linux-hwmon@vger.kernel.org
S: Orphan
diff --git a/drivers/firmware/coreboot/Kconfig b/drivers/firmware/coreboot/Kconfig
index b248bafe8b6b..5c37bdc9c9fd 100644
--- a/drivers/firmware/coreboot/Kconfig
+++ b/drivers/firmware/coreboot/Kconfig
@@ -44,6 +44,20 @@ config GOOGLE_COREBOOT_TABLE
device tree node /firmware/coreboot.
If unsure say N.
+config COREBOOT_CFR
+ tristate "coreboot CFR firmware attributes"
+ depends on EFI
+ depends on GOOGLE_COREBOOT_TABLE
+ select CRC32
+ select FW_ATTR_CLASS
+ help
+ This driver parses the coreboot CFR table and exposes runtime EFI
+ variable-backed options through the firmware-attributes sysfs
+ interface.
+
+ If you have a system running coreboot with CFR runtime
+ configuration support, say Y or M here.
+
config GOOGLE_MEMCONSOLE
tristate
depends on GOOGLE_MEMCONSOLE_X86_LEGACY || GOOGLE_MEMCONSOLE_COREBOOT
diff --git a/drivers/firmware/coreboot/Makefile b/drivers/firmware/coreboot/Makefile
index 8151e323cc43..1b9d0c0e7593 100644
--- a/drivers/firmware/coreboot/Makefile
+++ b/drivers/firmware/coreboot/Makefile
@@ -2,6 +2,7 @@
obj-$(CONFIG_GOOGLE_SMI) += gsmi.o
obj-$(CONFIG_GOOGLE_COREBOOT_TABLE) += coreboot_table.o
+obj-$(CONFIG_COREBOOT_CFR) += coreboot-cfr.o
obj-$(CONFIG_GOOGLE_FRAMEBUFFER_COREBOOT) += framebuffer-coreboot.o
obj-$(CONFIG_GOOGLE_MEMCONSOLE) += memconsole.o
obj-$(CONFIG_GOOGLE_MEMCONSOLE_COREBOOT) += memconsole-coreboot.o
diff --git a/drivers/firmware/coreboot/coreboot-cfr.c b/drivers/firmware/coreboot/coreboot-cfr.c
new file mode 100644
index 000000000000..ba9708a434c6
--- /dev/null
+++ b/drivers/firmware/coreboot/coreboot-cfr.c
@@ -0,0 +1,1141 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * coreboot CFR firmware attributes driver.
+ *
+ * Parses LB_TAG_CFR_ROOT records from the coreboot table and exposes
+ * runtime EFI variable-backed options through the firmware-attributes class.
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitops.h>
+#include <linux/cleanup.h>
+#include <linux/crc32.h>
+#include <linux/ctype.h>
+#include <linux/device.h>
+#include <linux/device-id/coreboot.h>
+#include <linux/efi.h>
+#include <linux/err.h>
+#include <linux/firmware_attributes.h>
+#include <linux/io.h>
+#include <linux/kdev_t.h>
+#include <linux/kobject.h>
+#include <linux/limits.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
+#include "coreboot_table.h"
+
+#define DRIVER_NAME "coreboot-cfr"
+
+#define LB_TAG_CFR_ROOT 0x47
+#define CFR_VERSION 0
+
+enum cfr_tags {
+ CFR_TAG_OPTION_FORM = 1,
+ CFR_TAG_ENUM_VALUE = 2,
+ CFR_TAG_OPTION_ENUM = 3,
+ CFR_TAG_OPTION_NUMBER = 4,
+ CFR_TAG_OPTION_BOOL = 5,
+ CFR_TAG_VARCHAR_OPT_NAME = 7,
+ CFR_TAG_VARCHAR_UI_NAME = 8,
+ CFR_TAG_RUNTIME_APPLY = 13,
+};
+
+enum cfr_option_flags {
+ CFR_OPTFLAG_READONLY = BIT(0),
+ CFR_OPTFLAG_INACTIVE = BIT(1),
+ CFR_OPTFLAG_SUPPRESS = BIT(2),
+ CFR_OPTFLAG_VOLATILE = BIT(3),
+ CFR_OPTFLAG_RUNTIME = BIT(4),
+};
+
+enum cfr_runtime_apply_method {
+ CFR_RUNTIME_APPLY_NONE = 0,
+ CFR_RUNTIME_APPLY_APM_CNT = 1,
+};
+
+struct lb_cfr {
+ u32 tag;
+ u32 size;
+ u32 version;
+ u32 checksum;
+} __packed;
+
+struct lb_cfr_varbinary {
+ u32 tag;
+ u32 size;
+ u32 data_length;
+} __packed;
+
+struct lb_cfr_enum_value {
+ u32 tag;
+ u32 size;
+ u32 value;
+} __packed;
+
+struct lb_cfr_runtime_apply {
+ u32 tag;
+ u32 size;
+ u32 method;
+ u32 id;
+} __packed;
+
+struct lb_cfr_numeric_option {
+ u32 tag;
+ u32 size;
+ cb_u64 object_id;
+ cb_u64 dependency_id;
+ u32 flags;
+ u32 default_value;
+ u32 min;
+ u32 max;
+ u32 step;
+ u32 display_flags;
+} __packed;
+
+struct lb_cfr_option_form {
+ u32 tag;
+ u32 size;
+ cb_u64 object_id;
+ cb_u64 dependency_id;
+ u32 flags;
+} __packed;
+
+#define COREBOOT_CFR_OPT_SKIP_FLAGS \
+ (CFR_OPTFLAG_SUPPRESS | CFR_OPTFLAG_VOLATILE)
+
+#define COREBOOT_CFR_OPT_READ_ONLY_FLAGS \
+ (CFR_OPTFLAG_READONLY | CFR_OPTFLAG_INACTIVE)
+
+#define COREBOOT_CFR_EFI_ATTRS \
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | \
+ EFI_VARIABLE_RUNTIME_ACCESS)
+
+#define COREBOOT_CFR_APM_CNT_PORT 0xb2
+#define COREBOOT_CFR_APM_STS_PORT 0xb3
+#define COREBOOT_CFR_APM_APPLY_CMD 0xe3
+#define COREBOOT_CFR_MAX_FORM_DEPTH 16
+
+static efi_guid_t coreboot_cfr_guid = EFI_GUID(0xceae4c1d, 0x335b, 0x4685,
+ 0xa4, 0xa0, 0xfc, 0x4a,
+ 0x94, 0xee, 0xa0, 0x85);
+
+enum coreboot_cfr_setting_type {
+ COREBOOT_CFR_SETTING_ENUM,
+ COREBOOT_CFR_SETTING_NUMBER,
+ COREBOOT_CFR_SETTING_BOOL,
+};
+
+struct coreboot_cfr_enum {
+ char *label;
+ u32 value;
+};
+
+struct coreboot_cfr_setting {
+ struct kobject kobj;
+ struct list_head node;
+ struct coreboot_cfr_drvdata *drvdata;
+ enum coreboot_cfr_setting_type type;
+ char *name;
+ char *display_name;
+ struct coreboot_cfr_enum *values;
+ unsigned int n_values;
+ u32 default_value;
+ u32 min;
+ u32 max;
+ u32 step;
+ u32 runtime_apply_method;
+ u32 runtime_apply_id;
+ bool read_only;
+};
+
+struct coreboot_cfr_drvdata {
+ struct device *class_dev;
+ struct kset *attrs_kset;
+ struct list_head settings;
+ /* Serializes EFI variable writes and the matching runtime apply hook. */
+ struct mutex lock;
+ bool efi_writes_supported;
+ bool pending_reboot;
+};
+
+static struct coreboot_cfr_setting *to_coreboot_cfr_setting(struct kobject *kobj)
+{
+ return container_of(kobj, struct coreboot_cfr_setting, kobj);
+}
+
+static bool coreboot_cfr_string_is_valid_name(const char *name)
+{
+ return name && name[0] && !strchr(name, '/');
+}
+
+static char *coreboot_cfr_string_dup(const struct lb_cfr_varbinary *str)
+{
+ const char *data = (const char *)(str + 1);
+ size_t len = str->data_length;
+
+ if (len && !data[len - 1])
+ len--;
+
+ return kmemdup_nul(data, len, GFP_KERNEL);
+}
+
+static const struct coreboot_table_entry *
+coreboot_cfr_child_entry(const void *base, size_t len, u32 tag)
+{
+ const struct coreboot_table_entry *entry;
+ size_t off;
+
+ for (off = 0; off < len; off += entry->size) {
+ if (len - off < sizeof(*entry))
+ return NULL;
+
+ entry = base + off;
+ if (entry->size < sizeof(*entry) || entry->size > len - off)
+ return NULL;
+
+ if (entry->tag == tag)
+ return entry;
+ }
+
+ return NULL;
+}
+
+static const struct lb_cfr_varbinary *
+coreboot_cfr_child_string(const void *base, size_t len, u32 tag)
+{
+ const struct lb_cfr_varbinary *str;
+ const struct coreboot_table_entry *entry;
+
+ entry = coreboot_cfr_child_entry(base, len, tag);
+ if (!entry)
+ return NULL;
+
+ if (entry->size < sizeof(*str))
+ return NULL;
+
+ str = (const struct lb_cfr_varbinary *)entry;
+ if (str->data_length > entry->size - sizeof(*str))
+ return NULL;
+
+ return str;
+}
+
+static const struct lb_cfr_runtime_apply *
+coreboot_cfr_child_runtime_apply(const void *base, size_t len)
+{
+ const struct lb_cfr_runtime_apply *runtime_apply;
+ const struct coreboot_table_entry *entry;
+
+ entry = coreboot_cfr_child_entry(base, len, CFR_TAG_RUNTIME_APPLY);
+ if (!entry)
+ return NULL;
+
+ if (entry->size < sizeof(*runtime_apply))
+ return ERR_PTR(-EINVAL);
+
+ runtime_apply = (const struct lb_cfr_runtime_apply *)entry;
+ if (runtime_apply->method == CFR_RUNTIME_APPLY_APM_CNT &&
+ runtime_apply->id > U8_MAX)
+ return ERR_PTR(-EINVAL);
+
+ return runtime_apply;
+}
+
+static efi_char16_t *coreboot_cfr_efi_name(const char *name)
+{
+ size_t len, i;
+
+ len = strlen(name);
+ if (len >= EFI_VAR_NAME_LEN)
+ return ERR_PTR(-ENAMETOOLONG);
+
+ efi_char16_t *efi_name __free(kfree) =
+ kcalloc(len + 1, sizeof(*efi_name), GFP_KERNEL);
+ if (!efi_name)
+ return ERR_PTR(-ENOMEM);
+
+ for (i = 0; i < len; i++) {
+ if (!isascii(name[i]))
+ return ERR_PTR(-EINVAL);
+ efi_name[i] = name[i];
+ }
+
+ return no_free_ptr(efi_name);
+}
+
+static int coreboot_cfr_read_efi_value_locked(efi_char16_t *efi_name,
+ u32 *value, u32 *attrs)
+{
+ unsigned long size = sizeof(__le32);
+ efi_status_t status;
+ __le32 data;
+ u32 attr;
+
+ status = efivar_get_variable(efi_name, &coreboot_cfr_guid, &attr,
+ &size, &data);
+ if (status != EFI_SUCCESS)
+ return efi_status_to_err(status);
+
+ if (size != sizeof(data))
+ return -EINVAL;
+
+ if (!(attr & EFI_VARIABLE_RUNTIME_ACCESS))
+ return -EOPNOTSUPP;
+
+ *value = le32_to_cpu(data);
+ if (attrs)
+ *attrs = attr;
+
+ return 0;
+}
+
+static int coreboot_cfr_read_value(const struct coreboot_cfr_setting *setting,
+ u32 *value, u32 *attrs)
+{
+ efi_char16_t *efi_name;
+ int ret;
+
+ efi_name = coreboot_cfr_efi_name(setting->name);
+ if (IS_ERR(efi_name))
+ return PTR_ERR(efi_name);
+
+ ret = efivar_lock();
+ if (ret) {
+ kfree(efi_name);
+ return ret;
+ }
+
+ ret = coreboot_cfr_read_efi_value_locked(efi_name, value, attrs);
+ efivar_unlock();
+ kfree(efi_name);
+
+ return ret;
+}
+
+static int coreboot_cfr_write_efi_value_locked(efi_char16_t *efi_name,
+ u32 value, u32 attrs)
+{
+ efi_status_t status;
+ __le32 data;
+
+ if (!efivar_supports_writes())
+ return -EROFS;
+
+ data = cpu_to_le32(value);
+ status = efivar_set_variable_locked(efi_name, &coreboot_cfr_guid, attrs,
+ sizeof(data), &data, false);
+ if (status != EFI_SUCCESS)
+ return efi_status_to_err(status);
+
+ return 0;
+}
+
+static int coreboot_cfr_apply_runtime(struct coreboot_cfr_setting *setting)
+{
+#ifdef CONFIG_X86
+ u8 status;
+
+ if (setting->runtime_apply_method != CFR_RUNTIME_APPLY_APM_CNT)
+ return -EOPNOTSUPP;
+
+ outb((u8)setting->runtime_apply_id, COREBOOT_CFR_APM_STS_PORT);
+ outb(COREBOOT_CFR_APM_APPLY_CMD, COREBOOT_CFR_APM_CNT_PORT);
+ status = inb(COREBOOT_CFR_APM_STS_PORT);
+ if (status)
+ return -EIO;
+
+ return 0;
+#else
+ return -EOPNOTSUPP;
+#endif
+}
+
+static int coreboot_cfr_write_value(struct coreboot_cfr_setting *setting,
+ u32 value)
+{
+ efi_char16_t *efi_name;
+ u32 attrs;
+ u32 old;
+ int restore_ret;
+ int ret;
+ bool changed = false;
+
+ if (setting->read_only)
+ return -EACCES;
+
+ efi_name = coreboot_cfr_efi_name(setting->name);
+ if (IS_ERR(efi_name))
+ return PTR_ERR(efi_name);
+
+ mutex_lock(&setting->drvdata->lock);
+ ret = efivar_lock();
+ if (ret)
+ goto out_unlock_mutex;
+
+ ret = coreboot_cfr_read_efi_value_locked(efi_name, &old, &attrs);
+ if (ret)
+ goto out_unlock_efi;
+
+ if ((attrs & COREBOOT_CFR_EFI_ATTRS) != COREBOOT_CFR_EFI_ATTRS) {
+ ret = -EOPNOTSUPP;
+ goto out_unlock_efi;
+ }
+
+ if (old == value)
+ goto out_unlock_efi;
+
+ ret = coreboot_cfr_write_efi_value_locked(efi_name, value, attrs);
+ if (ret)
+ goto out_unlock_efi;
+ changed = true;
+
+ ret = coreboot_cfr_apply_runtime(setting);
+ if (ret == -EOPNOTSUPP) {
+ /* EFI changed; firmware will consume it after reboot. */
+ setting->drvdata->pending_reboot = true;
+ ret = 0;
+ } else if (ret) {
+ restore_ret = coreboot_cfr_write_efi_value_locked(efi_name, old, attrs);
+ if (restore_ret) {
+ setting->drvdata->pending_reboot = true;
+ ret = restore_ret;
+ } else if (coreboot_cfr_apply_runtime(setting)) {
+ setting->drvdata->pending_reboot = true;
+ } else {
+ changed = false;
+ }
+ }
+
+out_unlock_efi:
+ efivar_unlock();
+out_unlock_mutex:
+ mutex_unlock(&setting->drvdata->lock);
+ kfree(efi_name);
+
+ if (changed)
+ kobject_uevent(&setting->drvdata->class_dev->kobj, KOBJ_CHANGE);
+ return ret;
+}
+
+static const char *
+coreboot_cfr_label_from_value(const struct coreboot_cfr_setting *setting,
+ u32 value)
+{
+ unsigned int i;
+
+ for (i = 0; i < setting->n_values; i++) {
+ if (setting->values[i].value == value)
+ return setting->values[i].label;
+ }
+
+ return NULL;
+}
+
+static int coreboot_cfr_parse_value(struct coreboot_cfr_setting *setting,
+ const char *label, u32 *value_out)
+{
+ unsigned int i;
+
+ for (i = 0; i < setting->n_values; i++) {
+ if (!sysfs_streq(label, setting->values[i].label))
+ continue;
+
+ *value_out = setting->values[i].value;
+ return 0;
+ }
+
+ return kstrtou32(label, 0, value_out);
+}
+
+static bool coreboot_cfr_value_is_valid(struct coreboot_cfr_setting *setting,
+ u32 value)
+{
+ u32 delta;
+
+ if (setting->type != COREBOOT_CFR_SETTING_NUMBER) {
+ /* Enum and bool values are valid only if they have a label. */
+ return coreboot_cfr_label_from_value(setting, value);
+ }
+
+ if (value < setting->min || value > setting->max)
+ return false;
+
+ if (!setting->step)
+ return true;
+
+ delta = value - setting->min;
+ return delta % setting->step == 0;
+}
+
+static ssize_t type_show(struct kobject *kobj, struct kobj_attribute *attr,
+ char *buf)
+{
+ struct coreboot_cfr_setting *setting = to_coreboot_cfr_setting(kobj);
+
+ if (setting->type == COREBOOT_CFR_SETTING_NUMBER)
+ return sysfs_emit(buf, "integer\n");
+
+ return sysfs_emit(buf, "enumeration\n");
+}
+
+static ssize_t display_name_language_code_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "en_US.UTF-8\n");
+}
+
+static ssize_t display_name_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct coreboot_cfr_setting *setting = to_coreboot_cfr_setting(kobj);
+
+ return sysfs_emit(buf, "%s\n", setting->display_name);
+}
+
+static ssize_t possible_values_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct coreboot_cfr_setting *setting = to_coreboot_cfr_setting(kobj);
+ ssize_t len = 0;
+ unsigned int i;
+
+ for (i = 0; i < setting->n_values; i++) {
+ len += sysfs_emit_at(buf, len, "%s%s", i ? ";" : "",
+ setting->values[i].label);
+ }
+
+ len += sysfs_emit_at(buf, len, "\n");
+ return len;
+}
+
+static ssize_t min_value_show(struct kobject *kobj, struct kobj_attribute *attr,
+ char *buf)
+{
+ struct coreboot_cfr_setting *setting = to_coreboot_cfr_setting(kobj);
+
+ return sysfs_emit(buf, "%u\n", setting->min);
+}
+
+static ssize_t max_value_show(struct kobject *kobj, struct kobj_attribute *attr,
+ char *buf)
+{
+ struct coreboot_cfr_setting *setting = to_coreboot_cfr_setting(kobj);
+
+ return sysfs_emit(buf, "%u\n", setting->max);
+}
+
+static ssize_t scalar_increment_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct coreboot_cfr_setting *setting = to_coreboot_cfr_setting(kobj);
+
+ return sysfs_emit(buf, "%u\n", setting->step);
+}
+
+static ssize_t default_value_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct coreboot_cfr_setting *setting = to_coreboot_cfr_setting(kobj);
+ const char *label;
+
+ if (setting->type == COREBOOT_CFR_SETTING_NUMBER)
+ return sysfs_emit(buf, "%u\n", setting->default_value);
+
+ label = coreboot_cfr_label_from_value(setting, setting->default_value);
+ if (!label)
+ return sysfs_emit(buf, "%u\n", setting->default_value);
+
+ return sysfs_emit(buf, "%s\n", label);
+}
+
+static ssize_t current_value_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct coreboot_cfr_setting *setting = to_coreboot_cfr_setting(kobj);
+ const char *label;
+ u32 value;
+ int ret;
+
+ ret = coreboot_cfr_read_value(setting, &value, NULL);
+ if (ret)
+ return ret;
+
+ if (setting->type == COREBOOT_CFR_SETTING_NUMBER)
+ return sysfs_emit(buf, "%u\n", value);
+
+ label = coreboot_cfr_label_from_value(setting, value);
+ if (!label)
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", label);
+}
+
+static ssize_t current_value_store(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct coreboot_cfr_setting *setting = to_coreboot_cfr_setting(kobj);
+ u32 value;
+ int ret;
+
+ ret = coreboot_cfr_parse_value(setting, buf, &value);
+ if (ret)
+ return ret;
+
+ if (!coreboot_cfr_value_is_valid(setting, value))
+ return -EINVAL;
+
+ ret = coreboot_cfr_write_value(setting, value);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static struct kobj_attribute type_attr = __ATTR_RO(type);
+static struct kobj_attribute display_name_language_code_attr =
+ __ATTR_RO(display_name_language_code);
+static struct kobj_attribute display_name_attr = __ATTR_RO(display_name);
+static struct kobj_attribute possible_values_attr = __ATTR_RO(possible_values);
+static struct kobj_attribute min_value_attr = __ATTR_RO(min_value);
+static struct kobj_attribute max_value_attr = __ATTR_RO(max_value);
+static struct kobj_attribute scalar_increment_attr = __ATTR_RO(scalar_increment);
+static struct kobj_attribute default_value_attr = __ATTR_RO(default_value);
+static struct kobj_attribute current_value_attr = __ATTR_RW(current_value);
+
+static struct attribute *coreboot_cfr_setting_attrs[] = {
+ &type_attr.attr,
+ &display_name_language_code_attr.attr,
+ &display_name_attr.attr,
+ &possible_values_attr.attr,
+ &min_value_attr.attr,
+ &max_value_attr.attr,
+ &scalar_increment_attr.attr,
+ &default_value_attr.attr,
+ ¤t_value_attr.attr,
+ NULL,
+};
+
+static umode_t coreboot_cfr_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int n)
+{
+ struct coreboot_cfr_setting *setting = to_coreboot_cfr_setting(kobj);
+
+ if (setting->type == COREBOOT_CFR_SETTING_NUMBER &&
+ attr == &possible_values_attr.attr)
+ return 0;
+
+ if (setting->type != COREBOOT_CFR_SETTING_NUMBER &&
+ (attr == &min_value_attr.attr || attr == &max_value_attr.attr ||
+ attr == &scalar_increment_attr.attr))
+ return 0;
+
+ if (setting->read_only && attr == ¤t_value_attr.attr)
+ return 0444;
+
+ return attr->mode;
+}
+
+static const struct attribute_group coreboot_cfr_setting_group = {
+ .attrs = coreboot_cfr_setting_attrs,
+ .is_visible = coreboot_cfr_attr_is_visible,
+};
+
+static void coreboot_cfr_free_setting(struct coreboot_cfr_setting *setting)
+{
+ unsigned int i;
+
+ for (i = 0; i < setting->n_values; i++)
+ kfree(setting->values[i].label);
+
+ kfree(setting->values);
+ kfree(setting->display_name);
+ kfree(setting->name);
+ kfree(setting);
+}
+
+static void coreboot_cfr_setting_release(struct kobject *kobj)
+{
+ coreboot_cfr_free_setting(to_coreboot_cfr_setting(kobj));
+}
+
+static const struct kobj_type coreboot_cfr_setting_ktype = {
+ .release = coreboot_cfr_setting_release,
+ .sysfs_ops = &kobj_sysfs_ops,
+};
+
+static ssize_t pending_reboot_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct coreboot_cfr_drvdata *data;
+
+ data = dev_get_drvdata(kobj_to_dev(kobj->parent));
+
+ return sysfs_emit(buf, "%d\n", data->pending_reboot);
+}
+
+static struct kobj_attribute pending_reboot_attr = __ATTR_RO(pending_reboot);
+
+static int coreboot_cfr_copy_bool_values(struct coreboot_cfr_setting *setting)
+{
+ static const struct coreboot_cfr_enum bool_values[] = {
+ { .label = "Disabled", .value = 0 },
+ { .label = "Enabled", .value = 1 },
+ };
+ unsigned int i;
+
+ setting->values = kcalloc(ARRAY_SIZE(bool_values), sizeof(*setting->values),
+ GFP_KERNEL);
+ if (!setting->values)
+ return -ENOMEM;
+
+ for (i = 0; i < ARRAY_SIZE(bool_values); i++) {
+ setting->values[i].label = kstrdup(bool_values[i].label,
+ GFP_KERNEL);
+ if (!setting->values[i].label)
+ return -ENOMEM;
+ setting->values[i].value = bool_values[i].value;
+ setting->n_values++;
+ }
+
+ return 0;
+}
+
+static int coreboot_cfr_count_enum_values(const void *base, size_t len)
+{
+ const struct coreboot_table_entry *entry;
+ size_t off;
+ int count = 0;
+
+ for (off = 0; off < len; off += entry->size) {
+ if (len - off < sizeof(*entry))
+ return -EINVAL;
+
+ entry = base + off;
+ if (entry->size < sizeof(*entry) || entry->size > len - off)
+ return -EINVAL;
+
+ if (entry->tag == CFR_TAG_ENUM_VALUE)
+ count++;
+ }
+
+ return count;
+}
+
+static int coreboot_cfr_copy_enum_values(struct coreboot_cfr_setting *setting,
+ const void *base, size_t len)
+{
+ const struct lb_cfr_enum_value *enum_value;
+ const struct lb_cfr_varbinary *label;
+ const struct coreboot_table_entry *entry;
+ size_t off;
+ int count;
+
+ count = coreboot_cfr_count_enum_values(base, len);
+ if (count <= 0)
+ return count ?: -EINVAL;
+
+ setting->values = kcalloc(count, sizeof(*setting->values), GFP_KERNEL);
+ if (!setting->values)
+ return -ENOMEM;
+
+ for (off = 0; off < len; off += entry->size) {
+ entry = base + off;
+ if (entry->tag != CFR_TAG_ENUM_VALUE)
+ continue;
+
+ if (entry->size < sizeof(*enum_value))
+ return -EINVAL;
+
+ enum_value = base + off;
+ label = coreboot_cfr_child_string(enum_value + 1,
+ enum_value->size - sizeof(*enum_value),
+ CFR_TAG_VARCHAR_UI_NAME);
+ if (!label)
+ return -EINVAL;
+
+ setting->values[setting->n_values].label =
+ coreboot_cfr_string_dup(label);
+ if (!setting->values[setting->n_values].label)
+ return -ENOMEM;
+
+ setting->values[setting->n_values].value = enum_value->value;
+ setting->n_values++;
+ }
+
+ return 0;
+}
+
+static bool
+coreboot_cfr_possible_values_fit(const struct coreboot_cfr_setting *setting)
+{
+ size_t len = 1; /* Trailing newline. */
+ unsigned int i;
+
+ for (i = 0; i < setting->n_values; i++) {
+ if (i)
+ len++;
+
+ if (strlen(setting->values[i].label) >= PAGE_SIZE - len)
+ return false;
+
+ len += strlen(setting->values[i].label);
+ }
+
+ return true;
+}
+
+static int coreboot_cfr_setting_is_usable(struct coreboot_cfr_setting *setting)
+{
+ u32 value;
+ int ret;
+
+ ret = coreboot_cfr_read_value(setting, &value, NULL);
+ if (ret)
+ return ret;
+
+ if (!coreboot_cfr_value_is_valid(setting, value))
+ return -EINVAL;
+
+ return 0;
+}
+
+static int coreboot_cfr_register_setting(struct coreboot_cfr_drvdata *data,
+ struct coreboot_cfr_setting *setting)
+{
+ int ret;
+
+ ret = kobject_init_and_add(&setting->kobj, &coreboot_cfr_setting_ktype,
+ &data->attrs_kset->kobj, "%s", setting->name);
+ if (ret) {
+ kobject_put(&setting->kobj);
+ return ret;
+ }
+
+ ret = sysfs_create_group(&setting->kobj, &coreboot_cfr_setting_group);
+ if (ret) {
+ kobject_put(&setting->kobj);
+ return ret;
+ }
+
+ list_add_tail(&setting->node, &data->settings);
+ return 0;
+}
+
+static int coreboot_cfr_add_numeric_option(struct coreboot_cfr_drvdata *data,
+ const struct lb_cfr_numeric_option *option,
+ bool parent_read_only)
+{
+ const struct lb_cfr_varbinary *name;
+ const struct lb_cfr_varbinary *display_name;
+ const struct lb_cfr_runtime_apply *runtime_apply;
+ const void *child_base = option + 1;
+ struct coreboot_cfr_setting *setting;
+ size_t child_len = option->size - sizeof(*option);
+ int ret;
+
+ if (!(option->flags & CFR_OPTFLAG_RUNTIME))
+ return 0;
+
+ if (option->flags & COREBOOT_CFR_OPT_SKIP_FLAGS)
+ return 0;
+
+ if (option->dependency_id)
+ return 0;
+
+ setting = kzalloc_obj(*setting, GFP_KERNEL);
+ if (!setting)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&setting->node);
+ setting->drvdata = data;
+ setting->default_value = option->default_value;
+ setting->min = option->min;
+ setting->max = option->max;
+ setting->step = option->step ?: 1;
+ setting->read_only =
+ (option->flags & COREBOOT_CFR_OPT_READ_ONLY_FLAGS) ||
+ !data->efi_writes_supported || parent_read_only;
+
+ runtime_apply = coreboot_cfr_child_runtime_apply(child_base, child_len);
+ if (IS_ERR(runtime_apply)) {
+ ret = PTR_ERR(runtime_apply);
+ goto err_put_setting;
+ }
+
+ if (runtime_apply && runtime_apply->method == CFR_RUNTIME_APPLY_APM_CNT) {
+ setting->runtime_apply_method = runtime_apply->method;
+ setting->runtime_apply_id = runtime_apply->id;
+ }
+
+ name = coreboot_cfr_child_string(child_base, child_len,
+ CFR_TAG_VARCHAR_OPT_NAME);
+ if (!name) {
+ ret = -EINVAL;
+ goto err_put_setting;
+ }
+
+ setting->name = coreboot_cfr_string_dup(name);
+ if (!setting->name) {
+ ret = -ENOMEM;
+ goto err_put_setting;
+ }
+
+ if (!coreboot_cfr_string_is_valid_name(setting->name)) {
+ ret = -EINVAL;
+ goto err_put_setting;
+ }
+
+ display_name = coreboot_cfr_child_string(child_base, child_len,
+ CFR_TAG_VARCHAR_UI_NAME);
+ if (display_name)
+ setting->display_name = coreboot_cfr_string_dup(display_name);
+ else
+ setting->display_name = kstrdup(setting->name, GFP_KERNEL);
+ if (!setting->display_name) {
+ ret = -ENOMEM;
+ goto err_put_setting;
+ }
+
+ switch (option->tag) {
+ case CFR_TAG_OPTION_BOOL:
+ setting->type = COREBOOT_CFR_SETTING_BOOL;
+ setting->min = 0;
+ setting->max = 1;
+ setting->step = 1;
+ ret = coreboot_cfr_copy_bool_values(setting);
+ break;
+ case CFR_TAG_OPTION_ENUM:
+ setting->type = COREBOOT_CFR_SETTING_ENUM;
+ ret = coreboot_cfr_copy_enum_values(setting, child_base,
+ child_len);
+ break;
+ case CFR_TAG_OPTION_NUMBER:
+ setting->type = COREBOOT_CFR_SETTING_NUMBER;
+ if (!setting->min && !setting->max)
+ setting->max = U32_MAX;
+ else if (setting->max < setting->min)
+ setting->max = U32_MAX;
+ ret = 0;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ if (ret)
+ goto err_put_setting;
+ if (setting->type != COREBOOT_CFR_SETTING_NUMBER &&
+ !coreboot_cfr_possible_values_fit(setting)) {
+ ret = 0;
+ goto err_put_setting;
+ }
+
+ ret = coreboot_cfr_setting_is_usable(setting);
+ if (ret) {
+ if (ret == -ENOENT || ret == -EINVAL || ret == -EOPNOTSUPP ||
+ ret == -ENAMETOOLONG)
+ ret = 0;
+ goto err_put_setting;
+ }
+
+ ret = coreboot_cfr_register_setting(data, setting);
+ if (ret)
+ return ret;
+
+ return 0;
+
+err_put_setting:
+ coreboot_cfr_free_setting(setting);
+ return ret;
+}
+
+static int coreboot_cfr_parse_records(struct coreboot_cfr_drvdata *data,
+ const void *base, size_t len,
+ unsigned int depth,
+ bool parent_read_only)
+{
+ const struct lb_cfr_option_form *form;
+ const struct coreboot_table_entry *entry;
+ const void *child_base;
+ size_t child_len;
+ size_t off;
+ int ret;
+
+ for (off = 0; off < len; off += entry->size) {
+ if (len - off < sizeof(*entry))
+ return -EINVAL;
+
+ entry = base + off;
+ if (entry->size < sizeof(*entry) || entry->size > len - off)
+ return -EINVAL;
+
+ switch (entry->tag) {
+ case CFR_TAG_OPTION_FORM:
+ if (entry->size < sizeof(struct lb_cfr_option_form))
+ return -EINVAL;
+
+ form = base + off;
+ if (form->dependency_id ||
+ form->flags & CFR_OPTFLAG_SUPPRESS)
+ break;
+
+ if (depth >= COREBOOT_CFR_MAX_FORM_DEPTH)
+ return -E2BIG;
+
+ child_base = base + off + sizeof(struct lb_cfr_option_form);
+ child_len = entry->size - sizeof(struct lb_cfr_option_form);
+ ret = coreboot_cfr_parse_records(data, child_base,
+ child_len, depth + 1,
+ parent_read_only ||
+ (form->flags &
+ COREBOOT_CFR_OPT_READ_ONLY_FLAGS));
+ if (ret)
+ return ret;
+ break;
+ case CFR_TAG_OPTION_ENUM:
+ case CFR_TAG_OPTION_NUMBER:
+ case CFR_TAG_OPTION_BOOL:
+ if (entry->size < sizeof(struct lb_cfr_numeric_option))
+ return -EINVAL;
+ ret = coreboot_cfr_add_numeric_option(data, base + off,
+ parent_read_only);
+ if (ret)
+ return ret;
+ break;
+ default:
+ /* Ignore unsupported, child-only and future record types. */
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static void coreboot_cfr_unregister_settings(struct coreboot_cfr_drvdata *data)
+{
+ struct coreboot_cfr_setting *setting, *tmp;
+
+ list_for_each_entry_safe(setting, tmp, &data->settings, node) {
+ sysfs_remove_group(&setting->kobj, &coreboot_cfr_setting_group);
+ list_del(&setting->node);
+ kobject_put(&setting->kobj);
+ }
+}
+
+static int coreboot_cfr_probe(struct coreboot_device *dev)
+{
+ const struct lb_cfr *root = (const struct lb_cfr *)dev->raw;
+ struct coreboot_cfr_drvdata *data;
+ size_t payload_len;
+ int ret;
+
+ if (dev->entry.size < sizeof(*root))
+ return -EINVAL;
+
+ if (root->tag != LB_TAG_CFR_ROOT || root->version != CFR_VERSION)
+ return -EINVAL;
+
+ if (root->size < sizeof(*root) || root->size > dev->entry.size)
+ return -EINVAL;
+
+ payload_len = root->size - sizeof(*root);
+ if (crc32_be(0, root + 1, payload_len) != root->checksum)
+ return -EBADMSG;
+
+ if (!efivar_is_available())
+ return -EPROBE_DEFER;
+
+ data = devm_kzalloc(&dev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ ret = efivar_lock();
+ if (ret)
+ return ret;
+ data->efi_writes_supported = efivar_supports_writes();
+ efivar_unlock();
+
+ INIT_LIST_HEAD(&data->settings);
+ ret = devm_mutex_init(&dev->dev, &data->lock);
+ if (ret)
+ return ret;
+
+ dev_set_drvdata(&dev->dev, data);
+
+ data->class_dev = device_create(&firmware_attributes_class, NULL,
+ MKDEV(0, 0), NULL, DRIVER_NAME);
+ if (IS_ERR(data->class_dev))
+ return PTR_ERR(data->class_dev);
+ dev_set_drvdata(data->class_dev, data);
+
+ data->attrs_kset = kset_create_and_add("attributes", NULL,
+ &data->class_dev->kobj);
+ if (!data->attrs_kset) {
+ ret = -ENOMEM;
+ goto err_unregister_dev;
+ }
+
+ ret = sysfs_create_file(&data->attrs_kset->kobj,
+ &pending_reboot_attr.attr);
+ if (ret)
+ goto err_unregister_attrs;
+
+ ret = coreboot_cfr_parse_records(data, root + 1, payload_len, 0, false);
+ if (ret)
+ goto err_unregister_settings;
+
+ if (list_empty(&data->settings)) {
+ ret = -ENODEV;
+ goto err_unregister_settings;
+ }
+
+ return 0;
+
+err_unregister_settings:
+ coreboot_cfr_unregister_settings(data);
+ sysfs_remove_file(&data->attrs_kset->kobj, &pending_reboot_attr.attr);
+err_unregister_attrs:
+ kset_unregister(data->attrs_kset);
+err_unregister_dev:
+ device_unregister(data->class_dev);
+ return ret;
+}
+
+static void coreboot_cfr_remove(struct coreboot_device *dev)
+{
+ struct coreboot_cfr_drvdata *data = dev_get_drvdata(&dev->dev);
+
+ coreboot_cfr_unregister_settings(data);
+ sysfs_remove_file(&data->attrs_kset->kobj, &pending_reboot_attr.attr);
+ kset_unregister(data->attrs_kset);
+ device_unregister(data->class_dev);
+}
+
+static const struct coreboot_device_id coreboot_cfr_ids[] = {
+ { .tag = LB_TAG_CFR_ROOT },
+ { }
+};
+MODULE_DEVICE_TABLE(coreboot, coreboot_cfr_ids);
+
+static struct coreboot_driver coreboot_cfr_driver = {
+ .probe = coreboot_cfr_probe,
+ .remove = coreboot_cfr_remove,
+ .drv = {
+ .name = DRIVER_NAME,
+ },
+ .id_table = coreboot_cfr_ids,
+};
+module_coreboot_driver(coreboot_cfr_driver);
+
+MODULE_AUTHOR("Sean Rhodes <sean@starlabs.systems>");
+MODULE_DESCRIPTION("coreboot CFR firmware attributes driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("EFIVAR");
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 2/3] firmware: Rename google firmware directory to coreboot
2026-07-17 8:50 ` [PATCH v7 2/3] firmware: Rename google firmware directory to coreboot Sean Rhodes
@ 2026-07-18 0:39 ` Julius Werner
0 siblings, 0 replies; 6+ messages in thread
From: Julius Werner @ 2026-07-18 0:39 UTC (permalink / raw)
To: Sean Rhodes
Cc: tzungbi, hansg, ilpo.jarvinen, corentin.chary, luke,
denis.benato, prasanth.ksr, jorge.lopez2, mpearson-lenovo,
derekjohn.clark, josh, briannorris, jwerner, tzimmermann,
javierm, kees, u.kleine-koenig, mst, chenhuacai, wenst,
florian.fainelli, titouan.ameline, oliver, linux-kernel,
platform-driver-x86, Dell.Client.Kernel, chrome-platform
> Keep the existing CONFIG_GOOGLE_* symbols to avoid user configuration
> churn in this mechanical move.
Is this standard kernel development practice? It seems a bit weird to
me to leave the config symbols mismatching the directory structure and
actual meaning of the setting. How long would we leave them mismatched
like this, forever?
If you want to prevent user config churn, an alternative option would
be to provide the old symbol names as transitional options, like this:
menuconfig COREBOOT_FIRMWARE
bool "coreboot firmware drivers"
default n
...
if COREBOOT_FIRMWARE
config COREBOOT_CBMEM
...
...
endif #COREBOOT_FIRMWARE
# Deprecated symbol names, do not use
menuconfig GOOGLE_FIRMWARE
bool
transitional
select COREBOOT_FIRMWARE
config GOOGLE_CBMEM
bool
transitional
select COREBOOT_CBMEM
...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 1/3] firmware: Move firmware attributes class helper
2026-07-17 8:50 ` [PATCH v7 1/3] firmware: Move firmware attributes class helper Sean Rhodes
@ 2026-07-18 2:22 ` Derek J. Clark
0 siblings, 0 replies; 6+ messages in thread
From: Derek J. Clark @ 2026-07-18 2:22 UTC (permalink / raw)
To: Sean Rhodes, tzungbi
Cc: hansg, ilpo.jarvinen, corentin.chary, luke, denis.benato,
prasanth.ksr, jorge.lopez2, mpearson-lenovo, josh, briannorris,
jwerner, tzimmermann, javierm, kees, u.kleine-koenig, mst,
chenhuacai, wenst, florian.fainelli, titouan.ameline, oliver,
linux-kernel, platform-driver-x86, Dell.Client.Kernel,
chrome-platform
On July 17, 2026 1:50:01 AM PDT, 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>
>Tested-by: Oliver Lin <oliver@liuxiaozhen.dev>
>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 | 2 +-
> drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 2 +-
> drivers/platform/x86/lenovo/think-lmi.c | 2 +-
> drivers/platform/x86/lenovo/wmi-other.c | 2 +-
> drivers/platform/x86/samsung-galaxybook.c | 2 +-
> .../linux/firmware_attributes.h | 6 +++---
> 13 files changed, 22 insertions(+), 15 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>
>
Hi Sean,
I think there is a middle ground between just renaming these while knowingly having them all in the wrong spot, and reorganizing all the includes for drivers whose includes aren't properly alphabetized in a single patch. One of these options would be preferable:
- Introduce a cleanup patch before the rename that does the reordering, then rename in the correct order.
- Do the rename without a full reordering, placing the firmware attributes include right after its preceding alphabetical order location so that it wouldn't need to be moved if someone else does a full reordering later.
Personally I think the first option is better, but if that's out of your desired scope or you're worried about additional effects in existing drivers then please at least do the second option.
Thanks,
Derek
> 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..93fac0d464d2 100644
>--- a/drivers/platform/x86/asus-armoury.c
>+++ b/drivers/platform/x86/asus-armoury.c
>@@ -34,7 +34,7 @@
> #include <linux/sysfs.h>
>
> #include "asus-armoury.h"
>-#include "firmware_attributes_class.h"
>+#include <linux/firmware_attributes.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..85670be01647 100644
>--- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
>+++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
>@@ -16,7 +16,7 @@
> #include <linux/sysfs.h>
> #include <linux/wmi.h>
> #include "dell-wmi-sysman.h"
>-#include "../../firmware_attributes_class.h"
>+#include <linux/firmware_attributes.h>
>
> #define MAX_TYPES 4
> #include <linux/nls.h>
>diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
>index 27fd6cd21529..44314ae4e5e3 100644
>--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
>+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
>@@ -14,7 +14,7 @@
> #include <linux/string.h>
> #include <linux/wmi.h>
> #include "bioscfg.h"
>-#include "../../firmware_attributes_class.h"
>+#include <linux/firmware_attributes.h>
> #include <linux/nls.h>
> #include <linux/errno.h>
>
>diff --git a/drivers/platform/x86/lenovo/think-lmi.c b/drivers/platform/x86/lenovo/think-lmi.c
>index e215e86e3db7..a56f152b5f28 100644
>--- a/drivers/platform/x86/lenovo/think-lmi.c
>+++ b/drivers/platform/x86/lenovo/think-lmi.c
>@@ -20,7 +20,7 @@
> #include <linux/types.h>
> #include <linux/dmi.h>
> #include <linux/wmi.h>
>-#include "../firmware_attributes_class.h"
>+#include <linux/firmware_attributes.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..e9a2c4684e5f 100644
>--- a/drivers/platform/x86/lenovo/wmi-other.c
>+++ b/drivers/platform/x86/lenovo/wmi-other.c
>@@ -50,7 +50,7 @@
> #include "wmi-capdata.h"
> #include "wmi-events.h"
> #include "wmi-helpers.h"
>-#include "../firmware_attributes_class.h"
>+#include <linux/firmware_attributes.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..74b8c887dd79 100644
>--- a/drivers/platform/x86/samsung-galaxybook.c
>+++ b/drivers/platform/x86/samsung-galaxybook.c
>@@ -28,7 +28,7 @@
> #include <linux/uuid.h>
> #include <linux/workqueue.h>
> #include <acpi/battery.h>
>-#include "firmware_attributes_class.h"
>+#include <linux/firmware_attributes.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 */
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-18 2:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-17 8:50 [PATCH v7 0/3] firmware: Add coreboot CFR firmware attributes driver Sean Rhodes
2026-07-17 8:50 ` [PATCH v7 1/3] firmware: Move firmware attributes class helper Sean Rhodes
2026-07-18 2:22 ` Derek J. Clark
2026-07-17 8:50 ` [PATCH v7 2/3] firmware: Rename google firmware directory to coreboot Sean Rhodes
2026-07-18 0:39 ` Julius Werner
2026-07-17 8:50 ` [PATCH v7 3/3] firmware: coreboot: Add CFR firmware attributes driver Sean Rhodes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox