[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250509-fw-attrs-api-v1-2-258afed65bfa@gmail.com>
Date: Fri, 09 May 2025 04:48:34 -0300
From: Kurt Borja <kuurtb@...il.com>
To: Hans de Goede <hdegoede@...hat.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
Thomas Weißschuh <linux@...ssschuh.net>,
Joshua Grisham <josh@...huagrisham.com>,
Mark Pearson <mpearson-lenovo@...ebb.ca>, Armin Wolf <W_Armin@....de>,
Mario Limonciello <mario.limonciello@....com>
Cc: Antheas Kapenekakis <lkml@...heas.dev>,
"Derek J. Clark" <derekjohn.clark@...il.com>,
Prasanth Ksr <prasanth.ksr@...l.com>, Jorge Lopez <jorge.lopez2@...com>,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
Dell.Client.Kernel@...l.com, Kurt Borja <kuurtb@...il.com>
Subject: [PATCH RFC 2/5] platform/x86: firmware_attributes_class: Add a
high level API
Add an attribute configuration mechanism through the newly introduced
`struct fwat_dev_config`, which makes use of other documented structs
and callbacks.
This API aims to be simple, yet flexible. In order to accomplish this,
the following features were taken into account:
* Ability to statically define attributes
* Custom read/write callbacks for each attribute type
* Ability to map attributes to numbers in order to differentiate them in
callbacks (`aux` number)
* Ability to reuse read/write callbacks in different attributes
* Ability to reuse property selection in different attributes
* Optional visibility callback for dynamic attribute visibility
Signed-off-by: Kurt Borja <kuurtb@...il.com>
---
drivers/platform/x86/firmware_attributes_class.c | 249 ++++++++++++++++++++++-
drivers/platform/x86/firmware_attributes_class.h | 228 +++++++++++++++++++++
2 files changed, 474 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/firmware_attributes_class.c b/drivers/platform/x86/firmware_attributes_class.c
index 58ab1495ba3bd449cfe17de2827a57a0c5937788..7cfb0f49f235728c7450a82a7e9d00b8963d3dea 100644
--- a/drivers/platform/x86/firmware_attributes_class.c
+++ b/drivers/platform/x86/firmware_attributes_class.c
@@ -7,14 +7,233 @@
#include <linux/kobject.h>
#include <linux/module.h>
#include <linux/slab.h>
+#include <linux/sysfs.h>
#include <linux/types.h>
#include "firmware_attributes_class.h"
+#define to_fwat_attribute_ext(_a) container_of_const(_a, struct fwat_attribute_ext, attr)
+
+struct fwat_attribute_ext {
+ struct fwat_attribute attr;
+ enum fwat_property prop;
+ const struct fwat_attr_config *config;
+};
+
const struct class firmware_attributes_class = {
.name = "firmware-attributes",
};
EXPORT_SYMBOL_GPL(firmware_attributes_class);
+static const char * const fwat_type_labels[] = {
+ [fwat_type_integer] = "integer",
+ [fwat_type_string] = "string",
+ [fwat_type_enumeration] = "enumeration",
+};
+
+static const char * const fwat_prop_labels[] = {
+ [FWAT_PROP_DISPLAY_NAME] = "display_name",
+ [FWAT_PROP_LANGUAGE_CODE] = "display_name_language_code",
+ [FWAT_PROP_DEFAULT] = "default",
+
+ [FWAT_INT_PROP_MIN] = "min_value",
+ [FWAT_INT_PROP_MAX] = "max_value",
+ [FWAT_INT_PROP_INCREMENT] = "scalar_increment",
+
+ [FWAT_STR_PROP_MIN] = "min_length",
+ [FWAT_STR_PROP_MAX] = "max_length",
+
+ [FWAT_ENUM_PROP_POSSIBLE_VALUES] = "possible_values",
+};
+
+static ssize_t
+fwat_type_show(struct device *dev, const struct fwat_attribute *attr, char *buf)
+{
+ const struct fwat_attribute_ext *ext = to_fwat_attribute_ext(attr);
+ const struct fwat_attr_config *config = ext->config;
+
+ return sysfs_emit(buf, "%s\n", fwat_type_labels[config->type]);
+}
+
+static ssize_t
+fwat_property_show(struct device *dev, const struct fwat_attribute *attr, char *buf)
+{
+ const struct fwat_attribute_ext *ext = to_fwat_attribute_ext(attr);
+ const struct fwat_attr_config *config = ext->config;
+
+ if (!config->ops->prop_read)
+ return -EOPNOTSUPP;
+
+ return config->ops->prop_read(dev, config->aux, ext->prop, buf);
+}
+
+static ssize_t
+fwat_current_value_show(struct device *dev, const struct fwat_attribute *attr, char *buf)
+{
+ const struct fwat_attribute_ext *ext = to_fwat_attribute_ext(attr);
+ const struct fwat_attr_config *config = ext->config;
+ const char *str;
+ long int_val;
+ int ret;
+
+ switch (config->type) {
+ case fwat_type_integer:
+ ret = config->ops->integer_read(dev, config->aux, &int_val);
+ if (ret)
+ return ret;
+
+ return sysfs_emit(buf, "%ld\n", int_val);
+ case fwat_type_string:
+ ret = config->ops->string_read(dev, config->aux, &str);
+ if (ret)
+ return ret;
+
+ return sysfs_emit(buf, "%s\n", str);
+ case fwat_type_enumeration:
+ ret = config->ops->enumeration_read(dev, config->aux, &str);
+ if (ret)
+ return ret;
+
+ return sysfs_emit(buf, "%s\n", str);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static ssize_t
+fwat_current_value_store(struct device *dev, const struct fwat_attribute *attr,
+ const char *buf, size_t count)
+{
+ const struct fwat_attribute_ext *ext = to_fwat_attribute_ext(attr);
+ const struct fwat_attr_config *config = ext->config;
+ long int_val;
+ int ret;
+
+ switch (config->type) {
+ case fwat_type_integer:
+ ret = kstrtol(buf, 0, &int_val);
+ if (ret)
+ return ret;
+
+ ret = config->ops->integer_write(dev, config->aux, int_val);
+ break;
+ case fwat_type_string:
+ ret = config->ops->string_write(dev, config->aux, buf);
+ break;
+ case fwat_type_enumeration:
+ ret = config->ops->enumeration_write(dev, config->aux, buf);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return ret ? ret : count;
+}
+
+static struct attribute *
+fwat_alloc_attr(struct device *dev, const struct fwat_attr_config *config,
+ const char *attr_name, umode_t mode, enum fwat_property prop,
+ ssize_t (*show)(struct device *dev, const struct fwat_attribute *attr,
+ char *buf),
+ ssize_t (*store)(struct device *dev, const struct fwat_attribute *attr,
+ const char *buf, size_t count))
+{
+ struct fwat_attribute_ext *fattr;
+
+ fattr = devm_kzalloc(dev, sizeof(*fattr), GFP_KERNEL);
+ if (!fattr)
+ return NULL;
+
+ fattr->attr.attr.name = attr_name;
+ fattr->attr.attr.mode = mode;
+ fattr->attr.show = show;
+ fattr->attr.store = store;
+ fattr->prop = prop;
+ fattr->config = config;
+ sysfs_attr_init(&fattr->attr.attr);
+
+ return &fattr->attr.attr;
+}
+
+static struct attribute **
+fwat_create_attrs(struct device *dev, const struct fwat_attr_config *config)
+{
+ struct attribute **attrs;
+ enum fwat_property prop;
+ unsigned int index = 0;
+
+ attrs = devm_kcalloc(dev, config->num_props + 3, sizeof(*attrs), GFP_KERNEL);
+ if (!attrs)
+ return NULL;
+
+ /*
+ * Create optional attributes
+ */
+ for (; index < config->num_props; index++) {
+ prop = config->props[index];
+ attrs[index] = fwat_alloc_attr(dev, config, fwat_prop_labels[prop],
+ 0444, prop, fwat_property_show, NULL);
+ }
+
+ /*
+ * Create mandatory attributes
+ */
+ attrs[index++] = fwat_alloc_attr(dev, config, "type", 0444, 0, fwat_type_show, NULL);
+ attrs[index++] = fwat_alloc_attr(dev, config, "current_value", 0644, 0,
+ fwat_current_value_show, fwat_current_value_store);
+
+ return attrs;
+}
+
+static const struct attribute_group *
+fwat_create_group(struct device *dev, const struct fwat_attr_config *config)
+{
+ struct attribute_group *group;
+ struct attribute **attrs;
+
+ group = devm_kzalloc(dev, sizeof(*group), GFP_KERNEL);
+ if (!group)
+ return NULL;
+
+ attrs = fwat_create_attrs(dev, config);
+ if (!attrs)
+ return NULL;
+
+ group->name = config->name;
+ group->attrs = attrs;
+
+ return group;
+}
+
+static const struct attribute_group **
+fwat_create_auto_groups(struct device *dev, const struct fwat_dev_config *config)
+{
+ const struct attribute_group **groups;
+ const struct attribute_group *grp;
+ unsigned int index = 0;
+ size_t ngroups = 0;
+
+ while (config->attrs_config[ngroups])
+ ngroups++;
+
+ groups = devm_kcalloc(dev, ngroups + 1, sizeof(*groups), GFP_KERNEL);
+ if (!groups)
+ return NULL;
+
+ for (unsigned int i = 0; i < ngroups; i++) {
+ if (config->is_visible &&
+ !config->is_visible(dev, config->attrs_config[i]))
+ continue;
+
+ grp = fwat_create_group(dev, config->attrs_config[i]);
+ if (!grp)
+ return NULL;
+
+ groups[index++] = grp;
+ }
+
+ return groups;
+}
+
static ssize_t fwat_attrs_kobj_show(struct kobject *kobj, struct attribute *attr,
char *buf)
{
@@ -61,6 +280,7 @@ static const struct kobj_type fwat_attrs_ktype = {
* device
* @parent: Parent device
* @name: Name of the class device
+ * @config: Device configuration
* @data: Drvdata of the class device
* @groups: Sysfs groups for the custom `fwat_attrs_ktype` kobj_type
*
@@ -72,8 +292,10 @@ static const struct kobj_type fwat_attrs_ktype = {
*/
struct fwat_device *
fwat_device_register(struct device *parent, const char *name, void *data,
+ const struct fwat_dev_config *config,
const struct attribute_group **groups)
{
+ const struct attribute_group **auto_groups;
struct fwat_device *fadev;
struct device *dev;
int ret;
@@ -97,19 +319,36 @@ fwat_device_register(struct device *parent, const char *name, void *data,
if (ret)
goto out_kobj_put;
- if (groups) {
- ret = sysfs_create_groups(&fadev->attrs_kobj, groups);
+ if (config) {
+ auto_groups = fwat_create_auto_groups(dev, config);
+ if (!auto_groups) {
+ ret = -ENOMEM;
+ goto out_kobj_unregister;
+ }
+
+ ret = sysfs_create_groups(&fadev->attrs_kobj, auto_groups);
if (ret)
goto out_kobj_unregister;
}
+ if (groups) {
+ ret = sysfs_create_groups(&fadev->attrs_kobj, groups);
+ if (ret)
+ goto out_remove_auto_groups;
+ }
+
fadev->dev = dev;
fadev->groups = groups;
+ fadev->auto_groups = groups;
kobject_uevent(&fadev->attrs_kobj, KOBJ_ADD);
return fadev;
+out_remove_auto_groups:
+ if (config)
+ sysfs_remove_groups(&fadev->attrs_kobj, auto_groups);
+
out_kobj_unregister:
kobject_del(&fadev->attrs_kobj);
@@ -125,6 +364,8 @@ void fwat_device_unregister(struct fwat_device *fwadev)
{
if (fwadev->groups)
sysfs_remove_groups(&fwadev->attrs_kobj, fwadev->groups);
+ if (fwadev->auto_groups)
+ sysfs_remove_groups(&fwadev->attrs_kobj, fwadev->auto_groups);
kobject_del(&fwadev->attrs_kobj);
kobject_put(&fwadev->attrs_kobj);
device_unregister(fwadev->dev);
@@ -143,6 +384,7 @@ static void devm_fwat_device_release(void *data)
* device
* @parent: Parent device
* @name: Name of the class device
+ * @config: Device configuration
* @data: Drvdata of the class device
* @groups: Sysfs groups for the custom `fwat_attrs_ktype` kobj_type
*
@@ -156,12 +398,13 @@ static void devm_fwat_device_release(void *data)
*/
struct fwat_device *
devm_fwat_device_register(struct device *parent, const char *name, void *data,
+ const struct fwat_dev_config *config,
const struct attribute_group **groups)
{
struct fwat_device *fadev;
int ret;
- fadev = fwat_device_register(parent, name, data, groups);
+ fadev = fwat_device_register(parent, name, data, config, groups);
if (IS_ERR(fadev))
return fadev;
diff --git a/drivers/platform/x86/firmware_attributes_class.h b/drivers/platform/x86/firmware_attributes_class.h
index ad94bf91e5af30a2b8feb9abf224ee6f0d17600a..f250875d785c3439682c43c693f98e1c20e9ff54 100644
--- a/drivers/platform/x86/firmware_attributes_class.h
+++ b/drivers/platform/x86/firmware_attributes_class.h
@@ -18,11 +18,14 @@ extern const struct class firmware_attributes_class;
* @dev: The class device.
* @attrs_kobj: The "attributes" root kobject.
* @groups: Sysfs groups attached to the @attrs_kobj.
+ * @auto_groups: Sysgs groups generated from &struct fwat_attr_config attached.
+ * to the @attrs_kobj
*/
struct fwat_device {
struct device *dev;
struct kobject attrs_kobj;
const struct attribute_group **groups;
+ const struct attribute_group **auto_groups;
};
#define to_fwat_device(_k) container_of_const(_k, struct fwat_device, attrs_kobj)
@@ -30,6 +33,7 @@ struct fwat_device {
/**
* struct fwat_attribute - The firmware-attributes's custom attribute
* @attr: Embedded struct attribute.
+ * @aux: Auxiliary number defined by the user.
* @show: Show method called by the "attributes" kobject's ktype.
* @store: Store method called by the "attributes" kobject's ktype.
*/
@@ -43,14 +47,238 @@ struct fwat_attribute {
#define to_fwat_attribute(_a) container_of_const(_a, struct fwat_attribute, attr)
+enum fwat_attr_type {
+ fwat_type_integer,
+ fwat_type_string,
+ fwat_type_enumeration,
+};
+
+enum fwat_property {
+ FWAT_PROP_DISPLAY_NAME,
+ FWAT_PROP_LANGUAGE_CODE,
+ FWAT_PROP_DEFAULT,
+
+ FWAT_INT_PROP_MIN,
+ FWAT_INT_PROP_MAX,
+ FWAT_INT_PROP_INCREMENT,
+
+ FWAT_STR_PROP_MIN,
+ FWAT_STR_PROP_MAX,
+
+ FWAT_ENUM_PROP_POSSIBLE_VALUES,
+};
+
+struct fwat_attr_config;
+
+/**
+ * struct fwat_attr_ops - Operations for a firmware *attribute*
+ * @prop_read: Callback for retrieving each configured property of an attribute.
+ * @integer_read: Callback for reading the current_value of an attribute of
+ * type *integer*.
+ * @integer_write: Callback for writing the current_value of an attribute of
+ * type *integer*.
+ * @string_read: Callback for reading the current_value of an attribute of type
+ * *string*.
+ * @string_write: Callback for writing the current_value of an attribute of type
+ * *string*.
+ * @enumeration_read: Callback for reading the current_value of an attribute of
+ * type *enumeration*.
+ * @enumeration_write: Callback for writing the current_value of an attribute
+ * of type *enumeration*.
+ */
+struct fwat_attr_ops {
+ ssize_t (*prop_read)(struct device *dev, long aux,
+ enum fwat_property prop, const char *buf);
+ union {
+ struct {
+ int (*integer_read)(struct device *dev, long aux,
+ long *val);
+ int (*integer_write)(struct device *dev, long aux,
+ long val);
+ };
+ struct {
+ int (*string_read)(struct device *dev, long aux,
+ const char **str);
+ int (*string_write)(struct device *dev, long aux,
+ const char *str);
+ };
+ struct {
+ int (*enumeration_read)(struct device *dev, long aux,
+ const char **str);
+ int (*enumeration_write)(struct device *dev, long aux,
+ const char *str);
+ };
+ };
+};
+
+/**
+ * struct fwat_attr_config - Configuration for a single firmware *attribute*
+ * @name: Name of the sysfs group associated with this *attribute*.
+ * @type: Type of this *attribute*.
+ * @aux: Auxiliary number defined by the user, which will be passed to
+ * read/write callbacks.
+ * @ops: Operations for this *attribute*.
+ * @props: Array of properties of this *attribute*.
+ * @num_props: Size of the props array.
+ */
+struct fwat_attr_config {
+ const char *name;
+ enum fwat_attr_type type;
+ long aux;
+ const struct fwat_attr_ops *ops;
+ const enum fwat_property *props;
+ size_t num_props;
+};
+
+/**
+ * DEFINE_SIMPLE_FWAT_OPS() - Define static &struct fwat_attr_ops for a simple
+ * *attribute* with no properties, i.e. No
+ * &fwat_attr_ops.read_prop callback
+ * @_name: Prefix of the `read` and `write` callbacks.
+ * @_type: Firmware *attribute* type.
+ *
+ * Example:
+ *
+ * static int example_read(...) {...}
+ * static int example_write(...) {...}
+ *
+ * DEFINE_SIMPLE_FWAT_OPS(example, ...);
+ */
+#define DEFINE_SIMPLE_FWAT_OPS(_name, _type) \
+ static const struct fwat_attr_ops _name##_ops = { \
+ ._type##_read = _name##_read, \
+ ._type##_write = _name##_write, \
+ }
+
+/**
+ * DEFINE_FWAT_OPS() - Define static &struct fwat_attr_ops with all callbacks.
+ * @_name: Prefix of the `read` and `write` callbacks.
+ * @_type: Firmware *attribute* type.
+ *
+ * Example:
+ *
+ * static int example_read(...) {...}
+ * static int example_write(...) {...}
+ * static int example_prop_read(...) {...}
+ *
+ * DEFINE_FWAT_OPS(example, ...);
+ */
+#define DEFINE_FWAT_OPS(_name, _type) \
+ static const struct fwat_attr_ops _name##_ops = { \
+ .prop_read = _name##_prop_read, \
+ ._type##_read = _name##_read, \
+ ._type##_write = _name##_write, \
+ }
+
+/**
+ * FWAT_CONFIG() - Configuration pointer for a single firmware *attribute*.
+ * @_name: String name of this *attribute*.
+ * @_type: Firmware *attribute* type.
+ * @_ops: Pointer to &struct fwat_attr_ops.
+ * @_props: Pointer to a enum fwat_property array.
+ * @_num_props: Size of the @_props array.
+ *
+ * This is a convenience macro to quickly construct a &struct fwat_attr_config
+ * array, which will be passed to &struct fwat_dev_config.
+ *
+ * Example:
+ *
+ * static int example_read(...) {...}
+ * static int example_write(...) {...}
+ * static int example_prop_read(...) {...}
+ *
+ * DEFINE_FWAT_OPS(example, ...);
+ *
+ * static const enum fwat_property props[] = {...};
+ *
+ * static const struct fwat_attr_config * const attrs_config[] = {
+ * FWAT_CONFIG(example, ..., &example_fwat_ops, props,
+ * ARRAY_SIZE(props)),
+ * ...
+ * NULL
+ * };
+ *
+ * static const struct fwat_dev_config fdev_config = {
+ * .attrs_config = attrs_config,
+ * ...
+ * }
+ */
+#define FWAT_CONFIG(_name, _type, _ops, _props, _num_props) \
+ (&(const struct fwat_attr_config) { \
+ .name = _name, \
+ .type = fwat_type_##_type, \
+ .ops = _ops, \
+ .num_props = _num_props, \
+ .props = _props, \
+ })
+
+/**
+ * FWAT_CONFIG_AUX() - Configuration pointer for a single firmware *attribute*
+ * with an auxiliary number defined by the user
+ * @_name: String name of this *attribute*.
+ * @_type: Firmware *attribute* type.
+ * @_aux: Auxiliary number defined by the user.
+ * @_ops: Pointer to &struct fwat_attr_ops.
+ * @_props: Pointer to a enum fwat_property array.
+ * @_num_props: Size of the @_props array.
+ *
+ * This is a convenience macro to quickly construct a &struct fwat_attr_config
+ * array, which will be passed to &struct fwat_dev_config.
+ *
+ * Example:
+ *
+ * static int example_read(...) {...}
+ * static int example_write(...) {...}
+ * static int example_prop_read(...) {...}
+ *
+ * DEFINE_FWAT_OPS(example, ...);
+ *
+ * static const enum fwat_property props[] = {...};
+ *
+ * static const struct fwat_attr_config * const config[] = {
+ * FWAT_CONFIG_AUX(example, ..., n, &example_fwat_ops, props,
+ * ARRAY_SIZE(props)),
+ * ...
+ * NULL
+ * };
+ *
+ * static const struct fwat_dev_config fdev_config = {
+ * .attrs_config = attrs_config,
+ * ...
+ * }
+ */
+#define FWAT_CONFIG_AUX(_name, _type, _aux, _ops, _props, _num_props) \
+ (&(const struct fwat_attr_config) { \
+ .name = _name, \
+ .type = fwat_type_##_type, \
+ .aux = _aux, \
+ .ops = _ops, \
+ .num_props = _num_props, \
+ .props = _props, \
+ })
+
+/**
+ * struct fwat_dev_config - Configuration for this devices's
+ * &fwat_device.auto_groups
+ * @attrs_config: NULL terminated &struct fwat_attr_config array.
+ * @is_visible: Optional visibility callback to determine the visibility
+ * of each auto_group.
+ */
+struct fwat_dev_config {
+ const struct fwat_attr_config *const *attrs_config;
+ bool (*is_visible)(struct device *dev, const struct fwat_attr_config *config);
+};
+
struct fwat_device * __must_check
fwat_device_register(struct device *parent, const char *name, void *data,
+ const struct fwat_dev_config *config,
const struct attribute_group **groups);
void fwat_device_unregister(struct fwat_device *fwadev);
struct fwat_device * __must_check
devm_fwat_device_register(struct device *parent, const char *name, void *data,
+ const struct fwat_dev_config *config,
const struct attribute_group **groups);
#endif /* FW_ATTR_CLASS_H */
--
2.49.0
Powered by blists - more mailing lists