power: supply: core: implement extension API

Various drivers, mostly in platform/x86 extend the ACPI battery driver
with additional sysfs attributes to implement more UAPIs than are
exposed through ACPI by using various side-channels, like WMI,
nonstandard ACPI or EC communication.

While the created sysfs attributes look similar to the attributes
provided by the powersupply core, there are various deficiencies:

* They don't show up in uevent payload.
* They can't be queried with the standard in-kernel APIs.
* They don't work with triggers.
* The extending driver has to reimplement all of the parsing,
formatting and sysfs display logic.
* Writing a extension driver is completely different from writing a
normal power supply driver.

This extension API avoids all of these issues.
An extension is just a "struct power_supply_ext" with the same kind of
callbacks as in a normal "struct power_supply_desc".

The API is meant to be used via battery_hook_register(), the same way as
the current extensions.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Link: https://lore.kernel.org/r/20241211-power-supply-extensions-v6-1-9d9dc3f3d387@weissschuh.net
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

diff --git a/drivers/power/supply/power_supply.h b/drivers/power/supply/power_supply.h
index 5dabbd895538003096b62d03fdd0201b82b090e6..531785516d2ac31f9a7f73a58e15e64cb81820ed 100644 (file)
--- a/drivers/power/supply/power_supply.h
+++ b/drivers/power/supply/power_supply.h
@@ -9,6 +9,8 @@
  *  Modified: 2004, Oct     Szabolcs Gyurko
  */
 
+#include <linux/lockdep.h>
+
 struct device;
 struct device_type;
 struct power_supply;
@@ -17,6 +19,21 @@ extern int power_supply_property_is_writeable(struct power_supply *psy,
                                              enum power_supply_property psp);
 extern bool power_supply_has_property(struct power_supply *psy,
                                      enum power_supply_property psp);
+extern bool power_supply_ext_has_property(const struct power_supply_ext *ext,
+                                         enum power_supply_property psp);
+
+struct power_supply_ext_registration {
+       struct list_head list_head;
+       const struct power_supply_ext *ext;
+       void *data;
+};
+
+/* Make sure that the macro is a single expression */
+#define power_supply_for_each_extension(pos, psy)                      \
+       if ( ({ lockdep_assert_held(&(psy)->extensions_sem); 0; }) )    \
+               ;                                                       \
+       else                                                            \
+               list_for_each_entry(pos, &(psy)->extensions, list_head) \
 
 #ifdef CONFIG_SYSFS
 

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 1db27afb7017c4428aa90372f7b9de03223671cf..29209c19ea670533d168f219797b656550523627 100644 (file)
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -80,6 +80,7 @@ static int __power_supply_changed_work(struct power_supply *pst, void *data)
 
 static void power_supply_changed_work(struct work_struct *work)
 {
+       int ret;
        unsigned long flags;
        struct power_supply *psy = container_of(work, struct power_supply,
                                                changed_work);
@@ -87,6 +88,16 @@ static void power_supply_changed_work(struct work_struct *work)
        dev_dbg(&psy->dev, "%s\n", __func__);
 
        spin_lock_irqsave(&psy->changed_lock, flags);
+
+       if (unlikely(psy->update_groups)) {
+               psy->update_groups = false;
+               spin_unlock_irqrestore(&psy->changed_lock, flags);
+               ret = sysfs_update_groups(&psy->dev.kobj, power_supply_dev_type.groups);
+               if (ret)
+                       dev_warn(&psy->dev, "failed to update sysfs groups: %pe\n", ERR_PTR(ret));
+               spin_lock_irqsave(&psy->changed_lock, flags);
+       }
+
        /*
         * Check 'changed' here to avoid issues due to race between
         * power_supply_changed() and this routine. In worst case
@@ -1208,15 +1219,34 @@ static bool psy_desc_has_property(const struct power_supply_desc *psy_desc,
        return found;
 }
 
+bool power_supply_ext_has_property(const struct power_supply_ext *psy_ext,
+                                  enum power_supply_property psp)
+{
+       int i;
+
+       for (i = 0; i < psy_ext->num_properties; i++)
+               if (psy_ext->properties[i] == psp)
+                       return true;
+
+       return false;
+}
+
 bool power_supply_has_property(struct power_supply *psy,
                               enum power_supply_property psp)
 {
+       struct power_supply_ext_registration *reg;
+
        if (psy_desc_has_property(psy->desc, psp))
                return true;
 
        if (power_supply_battery_info_has_prop(psy->battery_info, psp))
                return true;
 
+       power_supply_for_each_extension(reg, psy) {
+               if (power_supply_ext_has_property(reg->ext, psp))
+                       return true;
+       }
+
        return false;
 }
 
@@ -1224,12 +1254,21 @@ int power_supply_get_property(struct power_supply *psy,
                            enum power_supply_property psp,
                            union power_supply_propval *val)
 {
+       struct power_supply_ext_registration *reg;
+
        if (atomic_read(&psy->use_cnt) <= 0) {
                if (!psy->initialized)
                        return -EAGAIN;
                return -ENODEV;
        }
 
+       scoped_guard(rwsem_read, &psy->extensions_sem) {
+               power_supply_for_each_extension(reg, psy) {
+                       if (power_supply_ext_has_property(reg->ext, psp))
+                               return reg->ext->get_property(psy, reg->ext, reg->data, psp, val);
+               }
+       }
+
        if (psy_desc_has_property(psy->desc, psp))
                return psy->desc->get_property(psy, psp, val);
        else if (power_supply_battery_info_has_prop(psy->battery_info, psp))
@@ -1243,7 +1282,24 @@ int power_supply_set_property(struct power_supply *psy,
                            enum power_supply_property psp,
                            const union power_supply_propval *val)
 {
-       if (atomic_read(&psy->use_cnt) <= 0 || !psy->desc->set_property)
+       struct power_supply_ext_registration *reg;
+
+       if (atomic_read(&psy->use_cnt) <= 0)
+               return -ENODEV;
+
+       scoped_guard(rwsem_read, &psy->extensions_sem) {
+               power_supply_for_each_extension(reg, psy) {
+                       if (power_supply_ext_has_property(reg->ext, psp)) {
+                               if (reg->ext->set_property)
+                                       return reg->ext->set_property(psy, reg->ext, reg->data,
+                                                                     psp, val);
+                               else
+                                       return -ENODEV;
+                       }
+               }
+       }
+
+       if (!psy->desc->set_property)
                return -ENODEV;
 
        return psy->desc->set_property(psy, psp, val);
@@ -1253,7 +1309,22 @@ EXPORT_SYMBOL_GPL(power_supply_set_property);
 int power_supply_property_is_writeable(struct power_supply *psy,
                                        enum power_supply_property psp)
 {
-       return psy->desc->property_is_writeable && psy->desc->property_is_writeable(psy, psp);
+       struct power_supply_ext_registration *reg;
+
+       power_supply_for_each_extension(reg, psy) {
+               if (power_supply_ext_has_property(reg->ext, psp)) {
+                       if (reg->ext->property_is_writeable)
+                               return reg->ext->property_is_writeable(psy, reg->ext,
+                                                                      reg->data, psp);
+                       else
+                               return 0;
+               }
+       }
+
+       if (!psy->desc->property_is_writeable)
+               return 0;
+
+       return psy->desc->property_is_writeable(psy, psp);
 }
 
 void power_supply_external_power_changed(struct power_supply *psy)
@@ -1272,6 +1343,76 @@ int power_supply_powers(struct power_supply *psy, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(power_supply_powers);
 
+static int power_supply_update_sysfs_and_hwmon(struct power_supply *psy)
+{
+       unsigned long flags;
+
+       spin_lock_irqsave(&psy->changed_lock, flags);
+       psy->update_groups = true;
+       spin_unlock_irqrestore(&psy->changed_lock, flags);
+
+       power_supply_changed(psy);
+
+       power_supply_remove_hwmon_sysfs(psy);
+       return power_supply_add_hwmon_sysfs(psy);
+}
+
+int power_supply_register_extension(struct power_supply *psy, const struct power_supply_ext *ext,
+                                   void *data)
+{
+       struct power_supply_ext_registration *reg;
+       size_t i;
+       int ret;
+
+       if (!psy || !ext || !ext->properties || !ext->num_properties)
+               return -EINVAL;
+
+       guard(rwsem_write)(&psy->extensions_sem);
+
+       for (i = 0; i < ext->num_properties; i++)
+               if (power_supply_has_property(psy, ext->properties[i]))
+                       return -EEXIST;
+
+       reg = kmalloc(sizeof(*reg), GFP_KERNEL);
+       if (!reg)
+               return -ENOMEM;
+
+       reg->ext = ext;
+       reg->data = data;
+       list_add(&reg->list_head, &psy->extensions);
+
+       ret = power_supply_update_sysfs_and_hwmon(psy);
+       if (ret)
+               goto sysfs_hwmon_failed;
+
+       return 0;
+
+sysfs_hwmon_failed:
+       list_del(&reg->list_head);
+       kfree(reg);
+       return ret;
+}
+EXPORT_SYMBOL_GPL(power_supply_register_extension);
+
+void power_supply_unregister_extension(struct power_supply *psy, const struct power_supply_ext *ext)
+{
+       struct power_supply_ext_registration *reg;
+
+       guard(rwsem_write)(&psy->extensions_sem);
+
+       power_supply_for_each_extension(reg, psy) {
+               if (reg->ext == ext) {
+                       list_del(&reg->list_head);
+                       kfree(reg);
+                       power_supply_update_sysfs_and_hwmon(psy);
+                       return;
+               }
+       }
+
+       dev_warn(&psy->dev, "Trying to unregister invalid extension");
+}
+EXPORT_SYMBOL_GPL(power_supply_unregister_extension);
+
 static void power_supply_dev_release(struct device *dev)
 {
        struct power_supply *psy = to_power_supply(dev);
@@ -1426,6 +1567,9 @@ __power_supply_register(struct device *parent,
        }
 
        spin_lock_init(&psy->changed_lock);
+       init_rwsem(&psy->extensions_sem);
+       INIT_LIST_HEAD(&psy->extensions);
+
        rc = device_add(dev);
        if (rc)
                goto device_add_failed;
@@ -1438,13 +1582,15 @@ __power_supply_register(struct device *parent,
        if (rc)
                goto register_thermal_failed;
 
-       rc = power_supply_create_triggers(psy);
-       if (rc)
-               goto create_triggers_failed;
+       scoped_guard(rwsem_read, &psy->extensions_sem) {
+               rc = power_supply_create_triggers(psy);
+               if (rc)
+                       goto create_triggers_failed;
 
-       rc = power_supply_add_hwmon_sysfs(psy);
-       if (rc)
-               goto add_hwmon_sysfs_failed;
+               rc = power_supply_add_hwmon_sysfs(psy);
+               if (rc)
+                       goto add_hwmon_sysfs_failed;
+       }
 
        /*
         * Update use_cnt after any uevents (most notably from device_add()).

diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index e18f1ee53f2107c0d7a4ede829e57032c5f8b76e..b8f73bc89b05c6e68891c4c5352910afd6f94616 100644 (file)
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -299,6 +299,27 @@ static ssize_t power_supply_show_enum_with_available(
        return count;
 }
 
+static ssize_t power_supply_show_charge_behaviour(struct device *dev,
+                                                 struct power_supply *psy,
+                                                 union power_supply_propval *value,
+                                                 char *buf)
+{
+       struct power_supply_ext_registration *reg;
+
+       scoped_guard(rwsem_read, &psy->extensions_sem) {
+               power_supply_for_each_extension(reg, psy) {
+                       if (power_supply_ext_has_property(reg->ext,
+                                                         POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR))
+                               return power_supply_charge_behaviour_show(dev,
+                                               reg->ext->charge_behaviours,
+                                               value->intval, buf);
+               }
+       }
+
+       return power_supply_charge_behaviour_show(dev, psy->desc->charge_behaviours,
+                                                 value->intval, buf);
+}
+
 static ssize_t power_supply_format_property(struct device *dev,
                                            bool uevent,
                                            struct device_attribute *attr,
@@ -338,8 +359,7 @@ static ssize_t power_supply_format_property(struct device *dev,
        case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
                if (uevent) /* no possible values in uevents */
                        goto default_format;
-               ret = power_supply_charge_behaviour_show(dev, psy->desc->charge_behaviours,
-                                                        value.intval, buf);
+               ret = power_supply_show_charge_behaviour(dev, psy, &value, buf);
                break;
        case POWER_SUPPLY_PROP_CHARGE_TYPES:
                if (uevent) /* no possible values in uevents */
@@ -422,6 +442,8 @@ static umode_t power_supply_attr_is_visible(struct kobject *kobj,
        if (attrno == POWER_SUPPLY_PROP_TYPE)
                return mode;
 
+       guard(rwsem_read)(&psy->extensions_sem);
+
        if (power_supply_has_property(psy, attrno)) {
                if (power_supply_property_is_writeable(psy, attrno) > 0)
                        mode |= S_IWUSR;

diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 0d96657d1a2bcf9ce8a81e553aca7794406ac57e..a877518cd963d10cdcb2c2fc3a71a50a6e6098b8 100644 (file)
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -15,6 +15,8 @@
 #include <linux/device.h>
 #include <linux/workqueue.h>
 #include <linux/leds.h>
+#include <linux/rwsem.h>
+#include <linux/list.h>
 #include <linux/spinlock.h>
 #include <linux/notifier.h>
 
@@ -283,6 +285,27 @@ struct power_supply_desc {
        int use_for_apm;
 };
 
+struct power_supply_ext {
+       u8 charge_behaviours;
+       const enum power_supply_property *properties;
+       size_t num_properties;
+
+       int (*get_property)(struct power_supply *psy,
+                           const struct power_supply_ext *ext,
+                           void *data,
+                           enum power_supply_property psp,
+                           union power_supply_propval *val);
+       int (*set_property)(struct power_supply *psy,
+                           const struct power_supply_ext *ext,
+                           void *data,
+                           enum power_supply_property psp,
+                           const union power_supply_propval *val);
+       int (*property_is_writeable)(struct power_supply *psy,
+                                    const struct power_supply_ext *ext,
+                                    void *data,
+                                    enum power_supply_property psp);
+};
+
 struct power_supply {
        const struct power_supply_desc *desc;
 
@@ -302,10 +325,13 @@ struct power_supply {
        struct delayed_work deferred_register_work;
        spinlock_t changed_lock;
        bool changed;
+       bool update_groups;
        bool initialized;
        bool removing;
        atomic_t use_cnt;
        struct power_supply_battery_info *battery_info;
+       struct rw_semaphore extensions_sem; /* protects "extensions" */
+       struct list_head extensions;
 #ifdef CONFIG_THERMAL
        struct thermal_zone_device *tzd;
        struct thermal_cooling_device *tcd;
@@ -882,6 +908,13 @@ devm_power_supply_register(struct device *parent,
 extern void power_supply_unregister(struct power_supply *psy);
 extern int power_supply_powers(struct power_supply *psy, struct device *dev);
 
+extern int __must_check
+power_supply_register_extension(struct power_supply *psy,
+                               const struct power_supply_ext *ext,
+                               void *data);
+extern void power_supply_unregister_extension(struct power_supply *psy,
+                                             const struct power_supply_ext *ext);
+
 #define to_power_supply(device) container_of(device, struct power_supply, dev)
 
 extern void *power_supply_get_drvdata(struct power_supply *psy);