On Fri, May 04, 2007 at 01:31:39AM +0400, Anton Vorontsov wrote:
> 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
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.
> # cat /sys/class/power\ supply/
> ac/ main-battery/ usb/
Um, shouldn't that be an error? Isn't /sys/class/power\ supply/ a
directory?
> # 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
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?
>
> # 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?
>
> 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
>
> diff --git a/Documentation/power_supply_class.txt b/Documentation/power_supply_class.txt
> new file mode 100644
> index 0000000..666941f
> --- /dev/null
> +++ b/Documentation/power_supply_class.txt
> @@ -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.
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 050323f..c546de3 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -54,6 +54,8 @@ source "drivers/spi/Kconfig"
>
> source "drivers/w1/Kconfig"
>
> +source "drivers/power/Kconfig"
> +
> source "drivers/hwmon/Kconfig"
>
> source "drivers/mfd/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 3a718f5..5f41408 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -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/
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> new file mode 100644
> index 0000000..7811fa6
> --- /dev/null
> +++ b/drivers/power/Kconfig
> @@ -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
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> new file mode 100644
> index 0000000..95085ba
> --- /dev/null
> +++ b/drivers/power/Makefile
> @@ -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?
> +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
> diff --git a/drivers/power/power_supply.h b/drivers/power/power_supply.h
> new file mode 100644
> index 0000000..b1cb7c4
> --- /dev/null
> +++ b/drivers/power/power_supply.h
> @@ -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 */
> diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
> new file mode 100644
> index 0000000..6e28e47
> --- /dev/null
> +++ b/drivers/power/power_supply_core.c
> @@ -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)
> +{
> + flush_scheduled_work();
> + power_supply_remove_triggers(psy);
> + power_supply_remove_attrs(psy);
> + device_unregister(psy->dev);
> + return;
> +}
> +
> +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.
> --- /dev/null
> +++ b/drivers/power/power_supply_sysfs.c
> @@ -0,0 +1,254 @@
> +/*
> + * Sysfs interface for the universal power supply monitor class
> + *
> + * Copyright ?? 2007 David Woodhouse <[email protected]>
What's with the ??
> + * 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[] = {
> + "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?
> + 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),
> + 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?
> +
> +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,
> + &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.
> +
> + 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");
> +
> + 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.
> + 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,
> + &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?
> +
> +out:
> + free_page((unsigned long)prop_buf);
> +
> + return ret;
> +}
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
