Universal power supply class (was: battery class)

    # cat /sys/class/power\ supply/ac/type
        AC

        # cat /sys/class/power\ supply/usb/type
        USB 

Quoted 5 lines · Anton Vorontsov

    # cat /sys/class/power\ supply/ac/type
        AC

        # cat /sys/class/power\ supply/usb/type
        USB 

isnt that a bit redundant?

This class is result of "external power" and "battery" classes merge, as suggested by David Woodhouse. He also implemented uevent support.

Here how userspace seeing it now:

    	# ls /sys/class/power\ supply/
    	ac  main-battery  usb

    	# cat /sys/class/power\ supply/
    	ac/           main-battery/ usb/

    	# cat /sys/class/power\ supply/ac/type
    	AC

    	# cat /sys/class/power\ supply/usb/type
    	USB

    	# cat /sys/class/power\ supply/main-battery/type
    	Battery

    	# cat /sys/class/power\ supply/ac/online
    	1

    	# cat /sys/class/power\ supply/usb/online
    	0

    	# cat /sys/class/power\ supply/main-battery/status
    	Charging

    	# cat /sys/class/leds/h5400\:red-left/trigger
    	none h5400-radio timer hwtimer ac-online usb-online
    	main-battery-charging-or-full [main-battery-charging]
    	main-battery-full

Signed-off-by David Woodhouse <[email protected]>

Signed-off-by Anton Vorontsov <[email protected]>

---
 Documentation/power_supply_class.txt |  167 ++++++++++++++++++++++
 drivers/Kconfig                      |    2 +
 drivers/Makefile                     |    1 +
 drivers/power/Kconfig                |   17 +++
 drivers/power/Makefile               |   15 ++
 drivers/power/power_supply.h         |   42 ++++++
 drivers/power/power_supply_core.c    |  168 ++++++++++++++++++++++
 drivers/power/power_supply_leds.c    |  176 +++++++++++++++++++++++
 drivers/power/power_supply_sysfs.c   |  254 ++++++++++++++++++++++++++++++++++
 include/linux/power_supply.h         |  169 ++++++++++++++++++++++
 10 files changed, 1011 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/power_supply_class.txt
 create mode 100644 drivers/power/Kconfig
 create mode 100644 drivers/power/Makefile
 create mode 100644 drivers/power/power_supply.h
 create mode 100644 drivers/power/power_supply_core.c
 create mode 100644 drivers/power/power_supply_leds.c
 create mode 100644 drivers/power/power_supply_sysfs.c
 create mode 100644 include/linux/power_supply.h

Documentation/power_supply_class.txt new file +167 -0

new file mode 100644
index 0000000..666941f
@@ -0,0 +1,167 @@
+Linux power supply class
+========================
+
+Synopsis
+~~~~~~~~
+Power supply class used to represent battery, UPS, AC or DC power supply
+properties to user-space.
+
+It defines core set of attributes, which should be applicable to (almost)
+every power supply out there. Attributes are available via sysfs and uevent
+interfaces.
+
+Each attribute has well defined meaning, up to unit of measure used. While
+the attributes provided are believed to be universally applicable to any
+power supply, specific monitoring hardware may not be able to provide them
+all, so any of them may be skipped.
+
+Power supply class is extensible, and allows to define drivers own attributes.
+The core attribute set is subject to the standard Linux evolution (i.e.
+if it will be found that some attribute is applicable to many power supply
+types or their drivers, it can be added to the core set).
+
+It also integrates with LED framework, for the purpose of providing
+typically expected feedback of battery charging/fully charged status and
+AC/USB power supply online status. (Note that specific details of the
+indication (including whether to use it at all) are fully controllable by
+user and/or specific machine defaults, per design principles of LED
+framework).
+
+
+Attributes/properties
+~~~~~~~~~~~~~~~~~~~~~
+Power supply class has predefined set of attributes, this eliminates code
+duplication across drivers. Power supply class insist on reusing its
+predefined attributes *and* their units.
+
+So, userspace gets predictable set of attributes and their units for any
+kind of power supply, and can process/present them to a user in consistent
+manner. Results for different power supplies and machines are also directly
+comparable.
+
+See drivers/power/ds2760_battery.c and drivers/power/pda_power.c for the
+example how to declare and handle attributes.
+
+
+Units
+~~~~~
+Quoting include/linux/power_supply.h:
+
+  All voltages, currents, charges, energies, time and temperatures in uV,
+  uA, uAh, uWh, seconds and tenths of degree Celsius unless otherwise
+  stated. It's driver's job to convert its raw values to units in which
+  this class operates.
+
+
+Attributes/properties detailed
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+~ ~ ~ ~ ~ ~ ~  Charge/Energy/Capacity - how to not confuse  ~ ~ ~ ~ ~ ~ ~
+~                                                                       ~
+~ Because both "charge" (uAh) and "energy" (uWh) represents "capacity"  ~
+~ of battery, this class distinguish these terms. Don't mix them!       ~
+~                                                                       ~
+~ CHARGE_* attributes represents capacity in uAh only.                  ~
+~ ENERGY_* attributes represents capacity in uWh only.                  ~
+~ CAPACITY attribute represents capacity in *percents*, from 0 to 100.  ~
+~                                                                       ~
+~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~
+
+Postfixes:
+_AVG - *hardware* averaged value, use it if your hardware is really able to
+report averaged values.
+_NOW - momentary/instantaneous values.
+
+STATUS - this attribute represents operating status (charging, full,
+discharging (i.e. powering a load), etc.). This corresponds to
+BATTERY_STATUS_* values, as defined in battery.h.
+
+HEALTH - represents health of the battery, values corresponds to
+POWER_SUPPLY_HEALTH_*, defined in battery.h.
+
+VOLTAGE_MAX_DESIGN, VOLTAGE_MIN_DESIGN - design values for maximal and
+minimal power supply voltages. Maximal/minimal means values of voltages
+when battery considered "full"/"empty" at normal conditions. Yes, there is
+no direct relation between voltage and battery capacity, but some dumb
+batteries use voltage for very approximated calculation of capacity.
+Battery driver also can use this attribute just to inform userspace
+about maximal and minimal voltage thresholds of a given battery.
+
+CHARGE_FULL_DESIGN, CHARGE_EMPTY_DESIGN - design charge values, when
+battery considered full/empty.
+
+ENERGY_FULL_DESIGN, ENERGY_EMPTY_DESIGN - same as above but for energy.
+
+CHARGE_FULL, CHARGE_EMPTY - These attributes means "last remembered value
+of charge when battery became full/empty". It also could mean "value of
+charge when battery considered full/empty at given conditions (temperature,
+age)". I.e. these attributes represents real thresholds, not design values.
+
+ENERGY_FULL, ENERGY_EMPTY - same as above but for energy.
+
+CAPACITY - capacity in percents.
+CAPACITY_LEVEL - capacity level. This corresponds to
+POWER_SUPPLY_CAPACITY_LEVEL_*.
+
+TEMP - temperature of the power supply.
+TEMP_AMBIENT - ambient temperature.
+
+TIME_TO_EMPTY - seconds left for battery to be considered empty (i.e.
+while battery powers a load)
+TIME_TO_FULL - seconds left for battery to be considered full (i.e.
+while battery is charging)
+
+
+Battery <-> external power supply interaction
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Often power supplies are acting as supplies and supplicants at the same
+time. Batteries are good example. So, batteries usually care if they're
+externally powered or not.
+
+For that case, power supply class implements notification mechanism for
+batteries.
+
+External power supply (AC) lists supplicants (batteries) names in
+"supplied_to" struct member, and each power_supply_changed() call
+issued by external power supply will notify supplicants via
+external_power_changed callback.
+
+
+QA
+~~
+Q: Where is POWER_SUPPLY_PROP_XYZ attribute?
+A: If you cannot find attribute suitable for your driver needs, feel free
+   to add it and send patch along with your driver.
+
+   The attributes available currently are the ones currently provided by the
+   drivers written.
+
+   Good candidates to add in future: model/part#, cycle_time, manufacturer,
+   etc.
+
+
+Q: I have some very specific attribute (e.g. battery color), should I add
+   this attribute to standard ones?
+A: Most likely, no. Such attribute can be placed in the driver itself, if
+   it is useful. Of course, if the attribute in question applicable to
+   large set of batteries, provided by many drivers, and/or comes from
+   some general battery specification/standard, it may be a candidate to
+   be added to the core attribute set.
+
+
+Q: Suppose, my battery monitoring chip/firmware does not provides capacity
+   in percents, but provides charge_{now,full,empty}. Should I calculate
+   percentage capacity manually, inside the driver, and register CAPACITY
+   attribute? The same question about time_to_empty/time_to_full.
+A: Most likely, no. This class is designed to export properties which are
+   directly measurable by the specific hardware available.
+
+   Inferring not available properties using some heuristics or mathematical
+   model is not subject of work for a battery driver. Such functionality
+   should be factored out, and in fact, apm_power, the driver to serve
+   legacy APM API on top of power supply class, uses a simple heuristic of
+   approximating remaining battery capacity based on its charge, current,
+   voltage and so on. But full-fledged battery model is likely not subject
+   for kernel at all, as it would require floating point calculation to deal
+   with things like differential equations and Kalman filters. This is
+   better be handled by batteryd/libbattery, yet to be written.

drivers/Kconfig +2 -0

index 050323f..c546de3 100644
@@ -54,6 +54,8 @@ source "drivers/spi/Kconfig"
 
 source "drivers/w1/Kconfig"
 
+source "drivers/power/Kconfig"
+
 source "drivers/hwmon/Kconfig"
 
 source "drivers/mfd/Kconfig"

drivers/Makefile +1 -0

index 3a718f5..5f41408 100644
@@ -60,6 +60,7 @@ obj-$(CONFIG_I2O)		+= message/
 obj-$(CONFIG_RTC_LIB)		+= rtc/
 obj-$(CONFIG_I2C)		+= i2c/
 obj-$(CONFIG_W1)		+= w1/
+obj-$(CONFIG_POWER_SUPPLY)	+= power/
 obj-$(CONFIG_HWMON)		+= hwmon/
 obj-$(CONFIG_PHONE)		+= telephony/
 obj-$(CONFIG_MD)		+= md/

drivers/power/Kconfig new file +17 -0

new file mode 100644
index 0000000..7811fa6
@@ -0,0 +1,17 @@
+menuconfig POWER_SUPPLY
+	tristate "Power supply class support"
+	help
+	  Say Y here to enable power supply class support. This allows
+	  power supply (batteries, AC, USB) monitoring by userspace
+	  via sysfs and uevent (if available) and/or APM kernel interface
+	  (if selected below).
+
+if POWER_SUPPLY
+
+config POWER_SUPPLY_DEBUG
+	bool "Power supply debug"
+	help
+	  Say Y here to enable debugging messages for power supply class
+	  and drivers.
+
+endif # POWER_SUPPLY

drivers/power/Makefile new file +0 -0

new file mode 100644
index 0000000..95085ba

drivers/power/Makefile new file +9 -0

+ifeq ($(CONFIG_LEDS_TRIGGERS),y)
+power_supply-objs += power_supply_leds.o
+endif
+
+ifeq ($(CONFIG_POWER_SUPPLY_DEBUG),y)
+EXTRA_CFLAGS += -DDEBUG
+endif
+
+obj-$(CONFIG_POWER_SUPPLY)         += power_supply.o

drivers/power/power_supply.h new file +42 -0

new file mode 100644
index 0000000..b1cb7c4
@@ -0,0 +1,42 @@
+/*
+ *  Functions private to power supply class
+ *
+ *  Copyright (c) 2007  Anton Vorontsov <[email protected]>
+ *  Copyright (c) 2004  Szabolcs Gyurko
+ *  Copyright (c) 2003  Ian Molton <[email protected]>
+ *
+ *  Modified: 2004, Oct     Szabolcs Gyurko
+ *
+ *  You may use this code as per GPL version 2
+ */
+
+#ifdef CONFIG_SYSFS
+
+extern int power_supply_create_attrs(struct power_supply *psy);
+extern void power_supply_remove_attrs(struct power_supply *psy);
+extern int power_supply_uevent(struct device *dev, char **envp, int num_envp,
+                               char *buffer, int buffer_size);
+
+#else
+
+static inline int power_supply_create_attrs(struct power_supply *psy)
+{ return 0; }
+static inline void power_supply_remove_attrs(struct power_supply *psy) {}
+#define power_supply_uevent NULL
+
+#endif /* CONFIG_SYSFS */
+
+#ifdef CONFIG_LEDS_TRIGGERS
+
+extern void power_supply_update_leds(struct power_supply *psy);
+extern int power_supply_create_triggers(struct power_supply *psy);
+extern void power_supply_remove_triggers(struct power_supply *psy);
+
+#else
+
+static inline void power_supply_update_leds(struct power_supply *psy) {}
+static inline int power_supply_create_triggers(struct power_supply *psy)
+{ return 0; }
+static inline void power_supply_remove_triggers(struct power_supply *psy) {}
+
+#endif /* CONFIG_LEDS_TRIGGERS */

drivers/power/power_supply_core.c new file +127 -0

new file mode 100644
index 0000000..6e28e47
@@ -0,0 +1,168 @@
+/*
+ *  Universal power supply monitor class
+ *
+ *  Copyright (c) 2007  Anton Vorontsov <[email protected]>
+ *  Copyright (c) 2004  Szabolcs Gyurko
+ *  Copyright (c) 2003  Ian Molton <[email protected]>
+ *
+ *  Modified: 2004, Oct     Szabolcs Gyurko
+ *
+ *  You may use this code as per GPL version 2
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/power_supply.h>
+#include "power_supply.h"
+
+struct class *power_supply_class;
+
+static void power_supply_changed_work(struct work_struct *work)
+{
+	struct power_supply *psy = container_of(work, struct power_supply,
+	                                        changed_work);
+	int i;
+
+	dev_dbg(psy->dev, "%s\n", __FUNCTION__);
+
+	for (i = 0; i < psy->num_supplicants; i++) {
+		struct device *dev;
+
+		down(&power_supply_class->sem);
+		list_for_each_entry(dev, &power_supply_class->devices, node) {
+			struct power_supply *pst = dev_get_drvdata(dev);
+
+			if (!strcmp(psy->supplied_to[i], pst->name)) {
+				if (pst->external_power_changed)
+					pst->external_power_changed(pst);
+			}
+		}
+		up(&power_supply_class->sem);
+	}
+
+	power_supply_update_leds(psy);
+
+	kobject_uevent(&psy->dev->kobj, KOBJ_CHANGE);
+
+	return;
+}
+
+void power_supply_changed(struct power_supply *psy)
+{
+	dev_dbg(psy->dev, "%s\n", __FUNCTION__);
+
+	schedule_work(&psy->changed_work);
+
+	return;
+}
+
+int power_supply_am_i_supplied(struct power_supply *psy)
+{
+	union power_supply_propval ret = {0,};
+	struct device *dev;
+
+	down(&power_supply_class->sem);
+	list_for_each_entry(dev, &power_supply_class->devices, node) {
+		struct power_supply *epsy = dev_get_drvdata(dev);
+		int i;
+
+		for (i = 0; i < epsy->num_supplicants; i++) {
+			if (!strcmp(epsy->supplied_to[i], psy->name)) {
+				if (epsy->get_property(epsy,
+				          POWER_SUPPLY_PROP_ONLINE, &ret))
+					continue;
+				if (ret.intval)
+					goto out;
+			}
+		}
+	}
+out:
+	up(&power_supply_class->sem);
+
+	dev_dbg(psy->dev, "%s %d\n", __FUNCTION__, ret.intval);
+
+	return ret.intval;
+}
+
+int power_supply_register(struct device *parent, struct power_supply *psy)
+{
+	int rc = 0;
+
+	psy->dev = device_create(power_supply_class, parent, 0,
+	                         "%s", psy->name);
+	if (IS_ERR(psy->dev)) {
+		rc = PTR_ERR(psy->dev);
+		goto dev_create_failed;
+	}
+
+	dev_set_drvdata(psy->dev, psy);
+
+	INIT_WORK(&psy->changed_work, power_supply_changed_work);
+
+	rc = power_supply_create_attrs(psy);
+	if (rc)
+		goto create_attrs_failed;
+
+	rc = power_supply_create_triggers(psy);
+	if (rc)
+		goto create_triggers_failed;
+
+	power_supply_changed(psy);
+
+	goto success;
+
+create_triggers_failed:
+	power_supply_remove_attrs(psy);
+create_attrs_failed:
+	device_unregister(psy->dev);
+dev_create_failed:
+success:
+	return rc;
+}
+
+void power_supply_unregister(struct power_supply *psy)
+{

drivers/power/power_supply_sysfs.c new file +37 -0

+ *  Copyright (c) 2007  Anton Vorontsov <[email protected]>
+ *  Copyright (c) 2004  Szabolcs Gyurko
+ *  Copyright (c) 2003  Ian Molton <[email protected]>
+ *
+ *  Modified: 2004, Oct     Szabolcs Gyurko
+ *
+ *  You may use this code as per GPL version 2
+ */
+
+#include <linux/power_supply.h>
+
+/*
+ * This is because the name "current" breaks the device attr macro.
+ * The "current" word resolvs to "(get_current())" so instead of
+ * "current" "(get_current())" appears in the sysfs.
+ *
+ * The source of this definition is the device.h which calls __ATTR
+ * macro in sysfs.h which calls the __stringify macro.
+ *
+ * Only modification that the name is not tried to be resolved
+ * (as a macro let's say).
+ */
+
+#define POWER_SUPPLY_ATTR(_name)                                        \
+{                                                                       \
+	.attr = { .name = #_name, .mode = 0444, .owner = THIS_MODULE }, \
+	.show = power_supply_show_property,                             \
+	.store = NULL,                                                  \
+}
+
+static struct device_attribute power_supply_attrs[];
+
+static ssize_t power_supply_show_property(struct device *dev,
+                                          struct device_attribute *attr,
+                                          char *buf) {
+	static char *status_text[] = {
+		"Unknown", "Charging", "Discharging", "Not charging", "Full"

drivers/power/power_supply_sysfs.c new file +55 -0

+	ssize_t ret;
+	struct power_supply *psy = dev_get_drvdata(dev);
+	const off_t off = attr - power_supply_attrs;
+	union power_supply_propval value;
+
+	ret = psy->get_property(psy, off, &value);
+
+	if (ret < 0) {
+		dev_err(dev, "driver failed to report `%s' property\n",
+		        attr->attr.name);
+		return ret;
+	}
+
+	if (off == POWER_SUPPLY_PROP_STATUS)
+		return sprintf(buf, "%s\n", status_text[value.intval]);
+	else if (off == POWER_SUPPLY_PROP_HEALTH)
+		return sprintf(buf, "%s\n", health_text[value.intval]);
+	else if (off == POWER_SUPPLY_PROP_TECHNOLOGY)
+		return sprintf(buf, "%s\n", technology_text[value.intval]);
+	else if (off == POWER_SUPPLY_PROP_CAPACITY_LEVEL)
+		return sprintf(buf, "%s\n",
+		               capacity_level_text[value.intval]);
+	else if (off == POWER_SUPPLY_PROP_MODEL_NAME)
+		return sprintf(buf, "%s\n", value.strval);
+
+	return sprintf(buf, "%d\n", value.intval);
+}
+
+/* Must be in the same order as POWER_SUPPLY_PROP_* */
+static struct device_attribute power_supply_attrs[] = {
+	/* Properties of type `int' */
+	POWER_SUPPLY_ATTR(status),
+	POWER_SUPPLY_ATTR(health),
+	POWER_SUPPLY_ATTR(present),
+	POWER_SUPPLY_ATTR(online),
+	POWER_SUPPLY_ATTR(technology),
+	POWER_SUPPLY_ATTR(voltage_max_design),
+	POWER_SUPPLY_ATTR(voltage_min_design),
+	POWER_SUPPLY_ATTR(voltage_now),
+	POWER_SUPPLY_ATTR(voltage_avg),
+	POWER_SUPPLY_ATTR(current_now),
+	POWER_SUPPLY_ATTR(current_avg),
+	POWER_SUPPLY_ATTR(charge_full_design),
+	POWER_SUPPLY_ATTR(charge_empty_design),
+	POWER_SUPPLY_ATTR(charge_full),
+	POWER_SUPPLY_ATTR(charge_empty),
+	POWER_SUPPLY_ATTR(charge_now),
+	POWER_SUPPLY_ATTR(charge_avg),
+	POWER_SUPPLY_ATTR(energy_full_design),
+	POWER_SUPPLY_ATTR(energy_empty_design),
+	POWER_SUPPLY_ATTR(energy_full),
+	POWER_SUPPLY_ATTR(energy_empty),
+	POWER_SUPPLY_ATTR(energy_now),
+	POWER_SUPPLY_ATTR(energy_avg),
+	POWER_SUPPLY_ATTR(capacity),

drivers/power/power_supply_sysfs.c new file +22 -0

+
+static ssize_t power_supply_show_static_attrs(struct device *dev,
+                                              struct device_attribute *attr,
+                                              char *buf) {
+	static char *type_text[] = { "Battery", "UPS", "AC", "USB" };
+	struct power_supply *psy = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n", type_text[psy->type]);
+}
+
+static struct device_attribute power_supply_static_attrs[] = {
+	__ATTR(type, 0444, power_supply_show_static_attrs, NULL),
+};
+
+int power_supply_create_attrs(struct power_supply *psy)
+{
+	int rc = 0;
+	int i, j;
+
+	for (i = 0; i < ARRAY_SIZE(power_supply_static_attrs); i++) {
+		rc = device_create_file(psy->dev,
+		            &power_supply_static_attrs[i]);

drivers/power/power_supply_sysfs.c new file +38 -0

+
+	goto succeed;
+
+dynamics_failed:
+	while (j--)
+		device_remove_file(psy->dev,
+		           &power_supply_attrs[psy->properties[j]]);
+statics_failed:
+	while (i--)
+		device_remove_file(psy->dev,
+		           &power_supply_static_attrs[psy->properties[i]]);
+succeed:
+	return rc;
+}
+
+void power_supply_remove_attrs(struct power_supply *psy)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(power_supply_static_attrs); i++)
+		device_remove_file(psy->dev,
+		            &power_supply_static_attrs[i]);
+
+	for (i = 0; i < psy->num_properties; i++)
+		device_remove_file(psy->dev,
+		            &power_supply_attrs[psy->properties[i]]);
+
+	return;
+}
+
+int power_supply_uevent(struct device *dev, char **envp, int num_envp,
+                        char *buffer, int buffer_size)
+{
+	struct power_supply *psy = dev_get_drvdata(dev);
+	int i = 0, length = 0, ret = 0, j;
+	char *prop_buf;
+
+	dev_dbg(dev, "uevent\n");

drivers/power/power_supply_sysfs.c new file +42 -0

+	if (ret)
+		return ret;
+
+	prop_buf = (char *)get_zeroed_page(GFP_KERNEL);
+	if (!prop_buf)
+		return -ENOMEM;
+
+	for (j = 0; j < ARRAY_SIZE(power_supply_static_attrs); j++) {
+		struct device_attribute *attr;
+		char *line;
+
+		attr = &power_supply_static_attrs[j];
+
+		if (power_supply_show_static_attrs(dev, attr, prop_buf) < 0)
+			goto out;
+
+		line = strchr(prop_buf, '\n');
+		if (line)
+			*line = 0;
+
+		dev_dbg(dev, "Static prop %s=%s\n", attr->attr.name, prop_buf);
+
+		ret = add_uevent_var(envp, num_envp, &i, buffer, buffer_size,
+		                     &length, "POWER_SUPPLY_%s=%s",
+		                     attr->attr.name, prop_buf);
+		if (ret)
+			goto out;
+	}
+
+	dev_dbg(dev, "%d dynamic props\n", psy->num_properties);
+
+	for (j = 0; j < psy->num_properties; j++) {
+		struct device_attribute *attr;
+		char *line;
+
+		attr = &power_supply_attrs[psy->properties[j]];
+
+		if (power_supply_show_property(dev, attr, prop_buf) < 0)
+			goto out;
+
+		line = strchr(prop_buf, '\n');
+		if (line)

Quoted 2 lines

    	# cat /sys/class/power\ supply/
    	ac/           main-battery/ usb/

Um, shouldn't that be an error? Isn't /sys/class/power\ supply/ a directory?

Quoted 3 lines · Anton Vorontsov

This class is result of "external power" and "battery" classes merge, as suggested by David Woodhouse. He also implemented uevent support.

Quoted 4 lines

Here how userspace seeing it now:

    	# ls /sys/class/power\ supply/
    	ac  main-battery  usb

Please don't put a space in a class name. Yes, we can do it, but some scripts will bomb. If you look all of the other classes use a '_' instead.

Quoted 2 lines

    	# cat /sys/class/power\ supply/
    	ac/           main-battery/ usb/

Um, shouldn't that be an error? Isn't /sys/class/power\ supply/ a directory?

Quoted 12 lines

    	# cat /sys/class/power\ supply/ac/type
    	AC

    	# cat /sys/class/power\ supply/usb/type
    	USB

    	# cat /sys/class/power\ supply/main-battery/type
    	Battery

    	# cat /sys/class/power\ supply/ac/online
    	1

Quoted 2 lines

    	# cat /sys/class/power\ supply/usb/online
    	0

I don't really understand, is the 'usb' and 'ac' directories really a 'struct device' here? Shouldn't there be some symlinks around here?

Can you do a 'tree /sys/class/power\ supply/' and show me the output?

Quoted 8 lines

    	# cat /sys/class/power\ supply/main-battery/status
    	Charging

    	# cat /sys/class/leds/h5400\:red-left/trigger
    	none h5400-radio timer hwtimer ac-online usb-online
    	main-battery-charging-or-full [main-battery-charging]
    	main-battery-full

Huh? What does the led have to do with the battery?

Quoted 5 lines

drivers/power/Makefile new file +0 -0

new file mode 100644
index 0000000..95085ba

drivers/power/Makefile line 5

Quoted 6 lines

drivers/power/Makefile new file +5 -0

@@ -0,0 +1,15 @@
+power_supply-objs := power_supply_core.o
+
+ifeq ($(CONFIG_SYSFS),y)
+power_supply-objs += power_supply_sysfs.o
+endif

Why would this work at all without sysfs?

drivers/power/Makefile line 9

Quoted 4 lines

drivers/power/Makefile new file +4 -0

+
+static int __init power_supply_class_init(void)
+{
+	power_supply_class = class_create(THIS_MODULE, "power supply");

Please use "power_supply" instead as mentioned above.

drivers/power/power_supply_sysfs.c line 4

Quoted 7 lines

drivers/power/power_supply_sysfs.c new file +4 -0

@@ -0,0 +1,254 @@
+/*
+ *  Sysfs interface for the universal power supply monitor class
+ *
+ *  Copyright ??  2007  David Woodhouse <[email protected]>

What's with the ??

Quoted 42 lines

drivers/power/power_supply_sysfs.c new file +42 -0

+ *  Copyright (c) 2007  Anton Vorontsov <[email protected]>
+ *  Copyright (c) 2004  Szabolcs Gyurko
+ *  Copyright (c) 2003  Ian Molton <[email protected]>
+ *
+ *  Modified: 2004, Oct     Szabolcs Gyurko
+ *
+ *  You may use this code as per GPL version 2
+ */
+
+#include <linux/power_supply.h>
+
+/*
+ * This is because the name "current" breaks the device attr macro.
+ * The "current" word resolvs to "(get_current())" so instead of
+ * "current" "(get_current())" appears in the sysfs.
+ *
+ * The source of this definition is the device.h which calls __ATTR
+ * macro in sysfs.h which calls the __stringify macro.
+ *
+ * Only modification that the name is not tried to be resolved
+ * (as a macro let's say).
+ */
+
+#define POWER_SUPPLY_ATTR(_name)                                        \
+{                                                                       \
+	.attr = { .name = #_name, .mode = 0444, .owner = THIS_MODULE }, \
+	.show = power_supply_show_property,                             \
+	.store = NULL,                                                  \
+}
+
+static struct device_attribute power_supply_attrs[];
+
+static ssize_t power_supply_show_property(struct device *dev,
+                                          struct device_attribute *attr,
+                                          char *buf) {
+	static char *status_text[] = {
+		"Unknown", "Charging", "Discharging", "Not charging", "Full"
+	};
+	static char *health_text[] = {
+		"Unknown", "Good", "Overheat", "Dead"
+	};
+	static char *technology_text[] = {

drivers/power/power_supply_sysfs.c line 51

Quoted 5 lines

drivers/power/power_supply_sysfs.c new file +5 -0

+		"Unknown", "NiMH", "Li-ion", "Li-poly"
+	};
+	static char *capacity_level_text[] = {
+		"Unknown", "Critical", "Low", "Normal", "High", "Full"
+	};

Shouldn't these strings corrispond with some enumerated type somewhere? What is keeping them in sequence?

Quoted 59 lines

drivers/power/power_supply_sysfs.c new file +59 -0

+	ssize_t ret;
+	struct power_supply *psy = dev_get_drvdata(dev);
+	const off_t off = attr - power_supply_attrs;
+	union power_supply_propval value;
+
+	ret = psy->get_property(psy, off, &value);
+
+	if (ret < 0) {
+		dev_err(dev, "driver failed to report `%s' property\n",
+		        attr->attr.name);
+		return ret;
+	}
+
+	if (off == POWER_SUPPLY_PROP_STATUS)
+		return sprintf(buf, "%s\n", status_text[value.intval]);
+	else if (off == POWER_SUPPLY_PROP_HEALTH)
+		return sprintf(buf, "%s\n", health_text[value.intval]);
+	else if (off == POWER_SUPPLY_PROP_TECHNOLOGY)
+		return sprintf(buf, "%s\n", technology_text[value.intval]);
+	else if (off == POWER_SUPPLY_PROP_CAPACITY_LEVEL)
+		return sprintf(buf, "%s\n",
+		               capacity_level_text[value.intval]);
+	else if (off == POWER_SUPPLY_PROP_MODEL_NAME)
+		return sprintf(buf, "%s\n", value.strval);
+
+	return sprintf(buf, "%d\n", value.intval);
+}
+
+/* Must be in the same order as POWER_SUPPLY_PROP_* */
+static struct device_attribute power_supply_attrs[] = {
+	/* Properties of type `int' */
+	POWER_SUPPLY_ATTR(status),
+	POWER_SUPPLY_ATTR(health),
+	POWER_SUPPLY_ATTR(present),
+	POWER_SUPPLY_ATTR(online),
+	POWER_SUPPLY_ATTR(technology),
+	POWER_SUPPLY_ATTR(voltage_max_design),
+	POWER_SUPPLY_ATTR(voltage_min_design),
+	POWER_SUPPLY_ATTR(voltage_now),
+	POWER_SUPPLY_ATTR(voltage_avg),
+	POWER_SUPPLY_ATTR(current_now),
+	POWER_SUPPLY_ATTR(current_avg),
+	POWER_SUPPLY_ATTR(charge_full_design),
+	POWER_SUPPLY_ATTR(charge_empty_design),
+	POWER_SUPPLY_ATTR(charge_full),
+	POWER_SUPPLY_ATTR(charge_empty),
+	POWER_SUPPLY_ATTR(charge_now),
+	POWER_SUPPLY_ATTR(charge_avg),
+	POWER_SUPPLY_ATTR(energy_full_design),
+	POWER_SUPPLY_ATTR(energy_empty_design),
+	POWER_SUPPLY_ATTR(energy_full),
+	POWER_SUPPLY_ATTR(energy_empty),
+	POWER_SUPPLY_ATTR(energy_now),
+	POWER_SUPPLY_ATTR(energy_avg),
+	POWER_SUPPLY_ATTR(capacity),
+	POWER_SUPPLY_ATTR(capacity_level),
+	POWER_SUPPLY_ATTR(temp),
+	POWER_SUPPLY_ATTR(temp_ambient),
+	POWER_SUPPLY_ATTR(time_to_empty_now),

drivers/power/power_supply_sysfs.c line 116

Quoted 6 lines

drivers/power/power_supply_sysfs.c new file +6 -0

+	POWER_SUPPLY_ATTR(time_to_empty_avg),
+	POWER_SUPPLY_ATTR(time_to_full_now),
+	POWER_SUPPLY_ATTR(time_to_full_avg),
+	/* Properties of type `const char *' */
+	POWER_SUPPLY_ATTR(model_name),
+};

Don't you need a NULL attribute at the end here?

Quoted 28 lines

drivers/power/power_supply_sysfs.c new file +28 -0

+
+static ssize_t power_supply_show_static_attrs(struct device *dev,
+                                              struct device_attribute *attr,
+                                              char *buf) {
+	static char *type_text[] = { "Battery", "UPS", "AC", "USB" };
+	struct power_supply *psy = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n", type_text[psy->type]);
+}
+
+static struct device_attribute power_supply_static_attrs[] = {
+	__ATTR(type, 0444, power_supply_show_static_attrs, NULL),
+};
+
+int power_supply_create_attrs(struct power_supply *psy)
+{
+	int rc = 0;
+	int i, j;
+
+	for (i = 0; i < ARRAY_SIZE(power_supply_static_attrs); i++) {
+		rc = device_create_file(psy->dev,
+		            &power_supply_static_attrs[i]);
+		if (rc)
+			goto statics_failed;
+	}
+
+	for (j = 0; j < psy->num_properties; j++) {
+		rc = device_create_file(psy->dev,

drivers/power/power_supply_sysfs.c line 148

Quoted 4 lines

drivers/power/power_supply_sysfs.c new file +4 -0

+		            &power_supply_attrs[psy->properties[j]]);
+		if (rc)
+			goto dynamics_failed;
+	}

Why not use the default attribute field? If you do that, all of the logic in this function goes away as they are automatically created for you.

Quoted 38 lines

drivers/power/power_supply_sysfs.c new file +38 -0

+
+	goto succeed;
+
+dynamics_failed:
+	while (j--)
+		device_remove_file(psy->dev,
+		           &power_supply_attrs[psy->properties[j]]);
+statics_failed:
+	while (i--)
+		device_remove_file(psy->dev,
+		           &power_supply_static_attrs[psy->properties[i]]);
+succeed:
+	return rc;
+}
+
+void power_supply_remove_attrs(struct power_supply *psy)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(power_supply_static_attrs); i++)
+		device_remove_file(psy->dev,
+		            &power_supply_static_attrs[i]);
+
+	for (i = 0; i < psy->num_properties; i++)
+		device_remove_file(psy->dev,
+		            &power_supply_attrs[psy->properties[i]]);
+
+	return;
+}
+
+int power_supply_uevent(struct device *dev, char **envp, int num_envp,
+                        char *buffer, int buffer_size)
+{
+	struct power_supply *psy = dev_get_drvdata(dev);
+	int i = 0, length = 0, ret = 0, j;
+	char *prop_buf;
+
+	dev_dbg(dev, "uevent\n");

drivers/power/power_supply_sysfs.c line 196

Quoted 10 lines

drivers/power/power_supply_sysfs.c new file +10 -0

+
+	if (!psy) {
+		dev_dbg(dev, "No power supply yet\n");
+		return ret;
+	}
+
+	dev_dbg(dev, "POWER_SUPPLY_name=%s\n", psy->name);
+
+	ret = add_uevent_var(envp, num_envp, &i, buffer, buffer_size,
+	                     &length, "POWER_SUPPLY_name=%s", psy->name);

Why would POWER_SUPPLY_name be different from the kobject's name?

Also, environment variables are usually all in uppercase.

Quoted 47 lines

drivers/power/power_supply_sysfs.c new file +47 -0

+	if (ret)
+		return ret;
+
+	prop_buf = (char *)get_zeroed_page(GFP_KERNEL);
+	if (!prop_buf)
+		return -ENOMEM;
+
+	for (j = 0; j < ARRAY_SIZE(power_supply_static_attrs); j++) {
+		struct device_attribute *attr;
+		char *line;
+
+		attr = &power_supply_static_attrs[j];
+
+		if (power_supply_show_static_attrs(dev, attr, prop_buf) < 0)
+			goto out;
+
+		line = strchr(prop_buf, '\n');
+		if (line)
+			*line = 0;
+
+		dev_dbg(dev, "Static prop %s=%s\n", attr->attr.name, prop_buf);
+
+		ret = add_uevent_var(envp, num_envp, &i, buffer, buffer_size,
+		                     &length, "POWER_SUPPLY_%s=%s",
+		                     attr->attr.name, prop_buf);
+		if (ret)
+			goto out;
+	}
+
+	dev_dbg(dev, "%d dynamic props\n", psy->num_properties);
+
+	for (j = 0; j < psy->num_properties; j++) {
+		struct device_attribute *attr;
+		char *line;
+
+		attr = &power_supply_attrs[psy->properties[j]];
+
+		if (power_supply_show_property(dev, attr, prop_buf) < 0)
+			goto out;
+
+		line = strchr(prop_buf, '\n');
+		if (line)
+			*line = 0;
+
+		dev_dbg(dev, "prop %s=%s\n", attr->attr.name, prop_buf);
+
+		ret = add_uevent_var(envp, num_envp, &i, buffer, buffer_size,

drivers/power/power_supply_sysfs.c line 224

Quoted 5 lines

drivers/power/power_supply_sysfs.c new file +5 -0

+		                     &length, "POWER_SUPPLY_%s=%s",
+		                     attr->attr.name, prop_buf);
+		if (ret)
+			goto out;
+	}

We typically do not export _every_ attribute of a kobject to userspace through the kevent mechanism. Why is this needed?

drivers/power/power_supply_sysfs.c line 254

Quoted 6 lines

drivers/power/power_supply_sysfs.c new file +6 -0

+
+out:
+	free_page((unsigned long)prop_buf);
+
+	return ret;
+}

thanks,

greg k-h

This class is result of "external power" and "battery" classes merge, as suggested by David Woodhouse. He also implemented uevent support.

include/linux/power_supply.h line 99

Quoted 6 lines

include/linux/power_supply.h new file +6 -0

+enum power_supply_type {
+       POWER_SUPPLY_TYPE_BATTERY = 0,
+       POWER_SUPPLY_TYPE_UPS,
+       POWER_SUPPLY_TYPE_AC,
+       POWER_SUPPLY_TYPE_USB,
+};

How about dumb (non-USB) DC power? Any reason to distinguish it from AC?

Quoted 8 lines · Shem Multinymous

include/linux/power_supply.h line 99

Quoted 6 lines

include/linux/power_supply.h new file +6 -0

+enum power_supply_type {
+       POWER_SUPPLY_TYPE_BATTERY = 0,
+       POWER_SUPPLY_TYPE_UPS,
+       POWER_SUPPLY_TYPE_AC,
+       POWER_SUPPLY_TYPE_USB,
+};

How about dumb (non-USB) DC power? Any reason to distinguish it from AC?

Hmm, if it should not be distinguished, it is better to rename AC to something that means continuous power. But I'd rather have it AC and DC, as something might have both supplies separate, and you might want to differentiate them for some (human interface) reason. After all, USB and DC are not really different anyway...

Anyway, what IS the difference between UPS and battery, or UPS and AC/DC for that matter? When should UPS be used? If you have UPS there, should not MGG (motor-generator group) also be provided?

Given that USB-power *is* usually also "dumb" (i.e. it doesn't do any control signaling over the USB bus for power-control purposes), IMHO it might be better to have just battery, AC and DC as types. And a primary and secondary notion too, as that is common. It would be generic.

Or maybe I just didn't get the idea behind the "type" attribute :-)

I'd appreciate if these were documented in the text file.

Hello Henrique, Shem,

Quoted 25 lines · Henrique de Moraes Holschuh

Quoted 8 lines · Shem Multinymous

include/linux/power_supply.h line 99

Quoted 6 lines

include/linux/power_supply.h new file +6 -0

+enum power_supply_type {
+       POWER_SUPPLY_TYPE_BATTERY = 0,
+       POWER_SUPPLY_TYPE_UPS,
+       POWER_SUPPLY_TYPE_AC,
+       POWER_SUPPLY_TYPE_USB,
+};

How about dumb (non-USB) DC power? Any reason to distinguish it from AC?

Hmm, if it should not be distinguished, it is better to rename AC to something that means continuous power. But I'd rather have it AC and DC, as something might have both supplies separate, and you might want to differentiate them for some (human interface) reason. After all, USB and DC are not really different anyway...

Anyway, what IS the difference between UPS and battery, or UPS and AC/DC for that matter? When should UPS be used? If you have UPS there, should not MGG (motor-generator group) also be provided?

Given that USB-power *is* usually also "dumb" (i.e. it doesn't do any control signaling over the USB bus for power-control purposes), IMHO it might be better to have just battery, AC and DC as types. And a primary and secondary notion too, as that is common. It would be generic.

Quoted 3 lines

Or maybe I just didn't get the idea behind the "type" attribute :-)

I'd appreciate if these were documented in the text file.

I think I got the start point of confusion. It's indeed bad to call such power supply type as AC. Maybe I should it rename to ADAPTER? Or WALL?

type, is really type' of power supply: imagine icon GUI application will use for different types. Type is not alternating/direct current stuff, it will be better to impelemnt current_type' attribute for such matter.

As for Battery/UPS difference.. yes, they're quite similar.. but again, imagine laptop with battery, and connected to UPS. ;-) How userspace will differ internal battery from UPS? Yup, by type attribute. I hope it makes sense... If not, we simply can remove UPS type.

And again, much thanks for your review and ideas!

--
Anton Vorontsov
email: [email protected]
backup email: [email protected]
irc://irc.freenode.org/bd2
_______________________________________________
Kernel-discuss mailing list
[email protected]
https://handhelds.org/mailman/listinfo/kernel-discuss

Quoted 17 lines

Quoted 17 lines

Quoted 9 lines

include/linux/power_supply.h line 99

Quoted 6 lines

include/linux/power_supply.h new file +6 -0

+enum power_supply_type {
+       POWER_SUPPLY_TYPE_BATTERY = 0,
+       POWER_SUPPLY_TYPE_UPS,
+       POWER_SUPPLY_TYPE_AC,
+       POWER_SUPPLY_TYPE_USB,
+};

How about dumb (non-USB) DC power? Any reason to distinguish it from AC?

Hmm, if it should not be distinguished, it is better to rename AC to something that means continuous power. But I'd rather have it AC and DC, as something might have both supplies separate, and you might want to differentiate them for some (human interface) reason. After all, USB and DC are not really different anyway...

Quoted 4 lines

Quoted 4 lines

Anyway, what IS the difference between UPS and battery, or UPS and AC/DC for that matter? When should UPS be used? If you have UPS there, should not MGG (motor-generator group) also be provided?

Quoted 8 lines

Quoted 5 lines

secondary notion too, as that is common. It would be generic.

Or maybe I just didn't get the idea behind the "type" attribute :-)

I'd appreciate if these were documented in the text file.

I think I got the start point of confusion. It's indeed bad to call such power supply type as AC. Maybe I should it rename to ADAPTER? Or WALL?

or MAINS?

Hello Henrique, Shem,

Quoted 10 lines · Henrique de Moraes Holschuh

Quoted 8 lines · Shem Multinymous

include/linux/power_supply.h line 99

Quoted 6 lines

include/linux/power_supply.h new file +6 -0

+enum power_supply_type {
+       POWER_SUPPLY_TYPE_BATTERY = 0,
+       POWER_SUPPLY_TYPE_UPS,
+       POWER_SUPPLY_TYPE_AC,
+       POWER_SUPPLY_TYPE_USB,
+};

How about dumb (non-USB) DC power? Any reason to distinguish it from AC?

I think I got the start point of confusion. It's indeed bad to call such power supply type as AC.

Maybe I should it rename to ADAPTER? Or WALL?

There's actually a nice and terse term for that - mains, but I wonder how well non-English speaking people are familiar with it (I personally saw them only in manuals, which, as well known, nobody reads).

Hello Anton,

But in real world, it's rather common to colloquially name
such kind of power "AC", and that's "term" is known to many-many people, and intuitively understood even for those, who didn't hit it yet. Actually, this discussion is good confirmation that's the case - people understand in what meaning it is used here, and go to point that it's not "scientifically" correct.

Given that USB-power *is* usually also "dumb" (i.e. it doesn't do any control signaling over the USB bus for power-control purposes),

Quoted 2 lines · Henrique de Moraes Holschuh

Given that USB-power *is* usually also "dumb" (i.e. it doesn't do any control signaling over the USB bus for power-control purposes),

it might be dumb, but it is useful to know wether the PDA is charging from usb or mains power. and some devices allow one to switch on / off the ability to charge via usb

Quoted 3 lines · you

Quoted 2 lines · Henrique de Moraes Holschuh

Given that USB-power *is* usually also "dumb" (i.e. it doesn't do any control signaling over the USB bus for power-control purposes),

Quoted 3 lines

it might be dumb, but it is useful to know wether the PDA is charging from usb or mains power. and some devices allow one to switch on / off the ability to charge via usb

And USB does have power budgetting requirements, etc. (like
was already pointed in own of initial review messages). So, USB is definitely not the same thing as "dumb DC".

Quoted 5 lines · Paul Sokolovsky

Quoted 3 lines · you

Quoted 2 lines · Henrique de Moraes Holschuh

Given that USB-power *is* usually also "dumb" (i.e. it doesn't do any control signaling over the USB bus for power-control purposes),

Quoted 7 lines

Quoted 3 lines

it might be dumb, but it is useful to know wether the PDA is charging from usb or mains power. and some devices allow one to switch on / off the ability to charge via usb

And USB does have power budgetting requirements, etc. (like
was already pointed in own of initial review messages). So, USB is definitely not the same thing as "dumb DC".

Everything does. A dumb DC power source is a dumb DC power source. USB is no different here, *unless* it is using USB signaling to control the power link, at which point it is not a dumb DC power source anymore.

There really is no difference beween an AC brick, a DC brick, an AC/DC brick, or an USB port supplying DC power in a dumb way in a laptop or
handheld: they are all supposed-continous DC power supplies with a maximum power budget.

But the "GUI should show it differently" part does make sense. I am not completely convinced a high number of top-level types is the best way to go about it, though. I'd use subtypes, and a comprehensive enough set of standard types and sub-types, along with a description of exactly what they are to be used for. This will make it MUCH easier on the userspace side for GUI authors, and it will be much better for people to not use the wrong types when converting to the new class...

Quoted 2 lines · Henrique de Moraes Holschuh

Given that USB-power *is* usually also "dumb" (i.e. it doesn't do any control signaling over the USB bus for power-control purposes),

it might be dumb, but it is useful to know wether the PDA is charging from usb or mains power. and some devices allow one to switch on / off the ability to charge via usb

Quoted 8 lines · ian

Quoted 2 lines · Henrique de Moraes Holschuh

Given that USB-power *is* usually also "dumb" (i.e. it doesn't do any control signaling over the USB bus for power-control purposes),

it might be dumb, but it is useful to know wether the PDA is charging from usb or mains power. and some devices allow one to switch on / off the ability to charge via usb

Which, at the proper abstraction level provided by this class, means that it allows one to switch on or off a power supply channel.

Laptops let one do this with their batteries, too. It is the same thing. I didn't check if the class comes with an attribute for "enable/disable this power source", but if doesn't, we need to add one: it *is* a generic and widely used capability in laptops, and according to you, also on PDAs.