[PATCH 7/7] [RFC] APM emulation driver for class batteries

[PATCH 7/7] [RFC] APM emulation driver for class batteries

[Anton Vorontsov]

It finds battery with "main_battery" flag set (or with max_capacity if no
batteries marked as main), and converts battery values to APM form.

---
 drivers/battery/Kconfig     |    7 +++
 drivers/battery/Makefile    |    1 +
 drivers/battery/apm_power.c |  121 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 129 insertions(+), 0 deletions(-)
 create mode 100644 drivers/battery/apm_power.c

diff --git a/drivers/battery/Kconfig b/drivers/battery/Kconfig
index 0c14ae0..bbf8283 100644
--- a/drivers/battery/Kconfig
+++ b/drivers/battery/Kconfig
@@ -15,4 +15,11 @@ config BATTERY_DS2760
 	help
 	  Say Y here to enable support for batteries with ds2760 chip.
 
+config APM_POWER
+	tristate "APM emulation"
+	depends on BATTERY && APM
+	help
+	  Say Y here to enable support APM status emulation using
+	  battery class devices.
+
 endmenu
diff --git a/drivers/battery/Makefile b/drivers/battery/Makefile
index 9902513..cea5807 100644
--- a/drivers/battery/Makefile
+++ b/drivers/battery/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_BATTERY)              += battery.o
 obj-$(CONFIG_BATTERY_DS2760)       += ds2760_battery.o
+obj-$(CONFIG_APM_POWER)            += apm_power.o
diff --git a/drivers/battery/apm_power.c b/drivers/battery/apm_power.c
new file mode 100644
index 0000000..5e741c1
--- /dev/null
+++ b/drivers/battery/apm_power.c
@@ -0,0 +1,121 @@
+/*
+ * Copyright (c) 2007 Eugeny Boger
+ *
+ * Use consistent with the GNU GPL is permitted,
+ * provided that this copyright notice is
+ * preserved in its entirety in all copies and derived works.
+ */
+
+#include <linux/module.h>
+#include <linux/battery.h>
+#include <linux/apm-emulation.h>
+#include <linux/list.h>
+
+#define BATTERY_PROPERTY(property) (main_battery->get_##property ? \
+			main_battery->get_##property(main_battery) : 0)
+
+static struct battery *main_battery;
+
+static void (*old_apm_get_power_status)(struct apm_power_info*);
+
+static void apm_battery_find_main_battery(void)
+{
+	struct device *dev;
+	struct battery *bat, *batm;
+	int max_capacity = 0;
+
+	main_battery = NULL;
+	batm = NULL;
+	list_for_each_entry(dev, &battery_class->devices, node) {
+		bat = dev_get_drvdata(dev);
+		/* If none of battery devices cantains 'main_battery' flag,
+		   choice one with max capacity */
+		if (bat->get_max_capacity)
+			if (bat->get_max_capacity(bat) > max_capacity) {
+				batm = bat;
+				max_capacity = bat->get_max_capacity(bat);
+			}
+
+		if (bat->main_battery)
+			main_battery = bat;
+	}
+	if (!main_battery)
+		main_battery = batm;
+}
+
+static void apm_battery_apm_get_power_status(struct apm_power_info *info)
+{
+	int bat_current;
+
+	down(&battery_class->sem);
+	apm_battery_find_main_battery();
+	if (!main_battery) {
+		up(&battery_class->sem);
+		return;
+	}
+
+	if (BATTERY_PROPERTY(status) == BATTERY_STATUS_FULL)
+		info->battery_life = 100;
+	else
+		if (BATTERY_PROPERTY(max_capacity) -
+		                                BATTERY_PROPERTY(min_capacity))
+			info->battery_life = ((BATTERY_PROPERTY(capacity) -
+			             BATTERY_PROPERTY(min_capacity)) * 100) /
+			             (BATTERY_PROPERTY(max_capacity) -
+			              BATTERY_PROPERTY(min_capacity));
+		else
+			info->battery_life = -1;
+	if ((BATTERY_PROPERTY(status) == BATTERY_STATUS_CHARGING)
+	    || (BATTERY_PROPERTY(status) == BATTERY_STATUS_NOT_CHARGING)
+	    || (BATTERY_PROPERTY(status) == BATTERY_STATUS_FULL))
+		info->ac_line_status = APM_AC_ONLINE;
+	else
+		info->ac_line_status = APM_AC_OFFLINE;
+
+	if (BATTERY_PROPERTY(status) == BATTERY_STATUS_CHARGING)
+		info->battery_status = APM_BATTERY_STATUS_CHARGING;
+	else {
+		if (info->battery_life > 50)
+			info->battery_status = APM_BATTERY_STATUS_HIGH;
+		else if (info->battery_life > 5)
+			info->battery_status = APM_BATTERY_STATUS_LOW;
+		else
+			info->battery_status = APM_BATTERY_STATUS_CRITICAL;
+	}
+	info->battery_flag = info->battery_status;
+
+	bat_current = BATTERY_PROPERTY(current);
+	if (bat_current)
+		info->time = ((BATTERY_PROPERTY(capacity) - 
+		              BATTERY_PROPERTY(min_capacity)) * 60) /
+		             bat_current;
+	else
+		info->time = -1;
+
+	info->units = APM_UNITS_MINS;
+
+	up(&battery_class->sem);
+	return;
+}
+
+static int __init apm_battery_init(void)
+{
+	printk(KERN_INFO "APM Battery Driver\n");
+
+	old_apm_get_power_status = apm_get_power_status;
+	apm_get_power_status = apm_battery_apm_get_power_status;
+	return 0;
+}
+
+static void __exit apm_battery_exit(void)
+{
+	apm_get_power_status = old_apm_get_power_status;
+	return;
+}
+
+module_init(apm_battery_init);
+module_exit(apm_battery_exit);
+
+MODULE_AUTHOR("Eugeny Boger");
+MODULE_DESCRIPTION("APM emulation driver for battery monitoring class");
+MODULE_LICENSE("GPL");
-- 
1.5.0.5-dirty

Re: [PATCH 7/7] [RFC] APM emulation driver for class batteries

[Anton Vorontsov]

On Thu, Apr 12, 2007 at 03:26:44AM +0400, Anton Vorontsov wrote:
> It finds battery with "main_battery" flag set (or with max_capacity if no
> batteries marked as main), and converts battery values to APM form.

Changes:

- follows battery class changes

- minor cleanups


Subject: [PATCH] [take2] APM emulation driver for class batteries


Signed-off-by: Anton Vorontsov <cbou@mail.ru>
---
 drivers/battery/Kconfig     |    7 +++
 drivers/battery/Makefile    |    1 +
 drivers/battery/apm_power.c |  122 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 130 insertions(+), 0 deletions(-)
 create mode 100644 drivers/battery/apm_power.c

diff --git a/drivers/battery/Kconfig b/drivers/battery/Kconfig
index 0c14ae0..bbf8283 100644
--- a/drivers/battery/Kconfig
+++ b/drivers/battery/Kconfig
@@ -15,4 +15,11 @@ config BATTERY_DS2760
 	help
 	  Say Y here to enable support for batteries with ds2760 chip.
 
+config APM_POWER
+	tristate "APM emulation"
+	depends on BATTERY && APM
+	help
+	  Say Y here to enable support APM status emulation using
+	  battery class devices.
+
 endmenu
diff --git a/drivers/battery/Makefile b/drivers/battery/Makefile
index 9902513..cea5807 100644
--- a/drivers/battery/Makefile
+++ b/drivers/battery/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_BATTERY)              += battery.o
 obj-$(CONFIG_BATTERY_DS2760)       += ds2760_battery.o
+obj-$(CONFIG_APM_POWER)            += apm_power.o
diff --git a/drivers/battery/apm_power.c b/drivers/battery/apm_power.c
new file mode 100644
index 0000000..367a17d
--- /dev/null
+++ b/drivers/battery/apm_power.c
@@ -0,0 +1,122 @@
+/*
+ * Copyright (c) 2007 Eugeny Boger
+ *
+ * Use consistent with the GNU GPL is permitted,
+ * provided that this copyright notice is
+ * preserved in its entirety in all copies and derived works.
+ */
+
+#include <linux/module.h>
+#include <linux/battery.h>
+#include <linux/apm-emulation.h>
+
+#define BATTERY_PROP(bat, prop) ({                                 \
+	void *value = bat->get_property(bat, BATTERY_PROP_##prop); \
+	value ? *(int*)value : 0;                                  \
+})
+
+#define MBATTERY_PROP(prop) BATTERY_PROP(main_battery, prop)
+
+static struct battery *main_battery;
+
+static void (*old_apm_get_power_status)(struct apm_power_info*);
+
+static void apm_battery_find_main_battery(void)
+{
+	struct device *dev;
+	struct battery *bat, *batm;
+	int max_charge = 0;
+
+	main_battery = NULL;
+	batm = NULL;
+	list_for_each_entry(dev, &battery_class->devices, node) {
+		bat = dev_get_drvdata(dev);
+		/* If none of battery devices cantains 'main_battery' flag,
+		   choice one with max CHARGE */
+		if (BATTERY_PROP(bat, MAX_CHARGE) > max_charge) {
+			batm = bat;
+			max_charge = BATTERY_PROP(bat, MAX_CHARGE);
+		}
+
+		if (bat->main_battery)
+			main_battery = bat;
+	}
+	if (!main_battery)
+		main_battery = batm;
+}
+
+static void apm_battery_apm_get_power_status(struct apm_power_info *info)
+{
+	int bat_current;
+
+	down(&battery_class->sem);
+	apm_battery_find_main_battery();
+	if (!main_battery) {
+		up(&battery_class->sem);
+		return;
+	}
+
+	if (MBATTERY_PROP(STATUS) == BATTERY_STATUS_FULL)
+		info->battery_life = 100;
+	else if (MBATTERY_PROP(MAX_CHARGE) - MBATTERY_PROP(MIN_CHARGE))
+		info->battery_life = ((MBATTERY_PROP(CHARGE) -
+		                       MBATTERY_PROP(MIN_CHARGE)) * 100) /
+		                     (MBATTERY_PROP(MAX_CHARGE) -
+		                      MBATTERY_PROP(MIN_CHARGE));
+	else
+		info->battery_life = -1;
+
+	if ((MBATTERY_PROP(STATUS) == BATTERY_STATUS_CHARGING) ||
+	    (MBATTERY_PROP(STATUS) == BATTERY_STATUS_NOT_CHARGING) ||
+	    (MBATTERY_PROP(STATUS) == BATTERY_STATUS_FULL))
+		info->ac_line_status = APM_AC_ONLINE;
+	else
+		info->ac_line_status = APM_AC_OFFLINE;
+
+	if (MBATTERY_PROP(STATUS) == BATTERY_STATUS_CHARGING)
+		info->battery_status = APM_BATTERY_STATUS_CHARGING;
+	else {
+		if (info->battery_life > 50)
+			info->battery_status = APM_BATTERY_STATUS_HIGH;
+		else if (info->battery_life > 5)
+			info->battery_status = APM_BATTERY_STATUS_LOW;
+		else
+			info->battery_status = APM_BATTERY_STATUS_CRITICAL;
+	}
+	info->battery_flag = info->battery_status;
+
+	bat_current = MBATTERY_PROP(CURRENT);
+	if (bat_current)
+		info->time = ((MBATTERY_PROP(CHARGE) -
+		              MBATTERY_PROP(MIN_CHARGE)) * 60) /
+		             bat_current;
+	else
+		info->time = -1;
+
+	info->units = APM_UNITS_MINS;
+
+	up(&battery_class->sem);
+	return;
+}
+
+static int __init apm_battery_init(void)
+{
+	printk(KERN_INFO "APM Battery Driver\n");
+
+	old_apm_get_power_status = apm_get_power_status;
+	apm_get_power_status = apm_battery_apm_get_power_status;
+	return 0;
+}
+
+static void __exit apm_battery_exit(void)
+{
+	apm_get_power_status = old_apm_get_power_status;
+	return;
+}
+
+module_init(apm_battery_init);
+module_exit(apm_battery_exit);
+
+MODULE_AUTHOR("Eugeny Boger");
+MODULE_DESCRIPTION("APM emulation driver for battery monitoring class");
+MODULE_LICENSE("GPL");
-- 
1.5.0.5-dirty

Re: [PATCH 7/7] [RFC] APM emulation driver for class batteries

[Russell King]

On Fri, Apr 13, 2007 at 05:50:43PM +0400, Anton Vorontsov wrote:
> +static void (*old_apm_get_power_status)(struct apm_power_info*);
> +
> +static int __init apm_battery_init(void)
> +{
> +	printk(KERN_INFO "APM Battery Driver\n");
> +
> +	old_apm_get_power_status = apm_get_power_status;
> +	apm_get_power_status = apm_battery_apm_get_power_status;
> +	return 0;
> +}
> +
> +static void __exit apm_battery_exit(void)
> +{
> +	apm_get_power_status = old_apm_get_power_status;
> +	return;
> +}

Utterly unsafe.  What happens if some other module gets loaded which
does this, and then this module is unloaded followed by the other
module.  Result: Oops.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 7/7] [RFC] APM emulation driver for class batteries

[Anton Vorontsov]

On Mon, Apr 16, 2007 at 09:24:21PM +0100, Russell King wrote:
> On Fri, Apr 13, 2007 at 05:50:43PM +0400, Anton Vorontsov wrote:
> > +static void (*old_apm_get_power_status)(struct apm_power_info*);
> > +
> > +static int __init apm_battery_init(void)
> > +{
> > +	printk(KERN_INFO "APM Battery Driver\n");
> > +
> > +	old_apm_get_power_status = apm_get_power_status;
> > +	apm_get_power_status = apm_battery_apm_get_power_status;
> > +	return 0;
> > +}
> > +
> > +static void __exit apm_battery_exit(void)
> > +{
> > +	apm_get_power_status = old_apm_get_power_status;
> > +	return;
> > +}
> 
> Utterly unsafe.  What happens if some other module gets loaded which
> does this, and then this module is unloaded followed by the other
> module.  Result: Oops.

Right. And loading two modules which changing apm_get_power_status
is a race already. Thus, APM interface needs a mutex.

Or pda_power should be marked "bool" in Kconfig, as it is done
in arch/arm/common/sharpsl_pm.c. Sharpsl_pm is safe only because it
can't be a module.

Personally I'd keep things as is for now (i.e. I'd want tristate for
PDA_POWER, not bool). Later APM API can be fixed.

> -- 
> Russell King
>  Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
>  maintainer of:
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.org/bd2

Re: [PATCH 7/7] [RFC] APM emulation driver for class batteries

[Russell King]

On Tue, Apr 17, 2007 at 01:08:29AM +0400, Anton Vorontsov wrote:
> On Mon, Apr 16, 2007 at 09:24:21PM +0100, Russell King wrote:
> > Utterly unsafe.  What happens if some other module gets loaded which
> > does this, and then this module is unloaded followed by the other
> > module.  Result: Oops.
> 
> Right. And loading two modules which changing apm_get_power_status
> is a race already. Thus, APM interface needs a mutex.
> 
> Or pda_power should be marked "bool" in Kconfig, as it is done
> in arch/arm/common/sharpsl_pm.c. Sharpsl_pm is safe only because it
> can't be a module.
> 
> Personally I'd keep things as is for now (i.e. I'd want tristate for
> PDA_POWER, not bool). Later APM API can be fixed.

Experience shows "Later" more often than not means "never", inspite
of what is said at the time the word is used...

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [Kernel-discuss] Re: [PATCH 7/7] [RFC] APM emulation driver for class batteries

[Paul Sokolovsky]

Hello Russell,

Monday, April 16, 2007, 11:24:21 PM, you wrote:

> On Fri, Apr 13, 2007 at 05:50:43PM +0400, Anton Vorontsov wrote:
>> +static void (*old_apm_get_power_status)(struct apm_power_info*);
>> +
>> +static int __init apm_battery_init(void)
>> +{
>> +     printk(KERN_INFO "APM Battery Driver\n");
>> +
>> +     old_apm_get_power_status = apm_get_power_status;
>> +     apm_get_power_status = apm_battery_apm_get_power_status;
>> +     return 0;
>> +}
>> +
>> +static void __exit apm_battery_exit(void)
>> +{
>> +     apm_get_power_status = old_apm_get_power_status;
>> +     return;
>> +}

> Utterly unsafe.  What happens if some other module gets loaded which
> does this, and then this module is unloaded followed by the other
> module.  Result: Oops.

	That's apparently why "APM emulation" goes on its way towards deprecation, right? And why people so detailed about new battery API, as it's everyone's hope that it should replace APM.

	We exactly provide APM emulation on top of battery API as separate driver because of such issues with APM API. Anyway, any suggestions on solving this "pointer API" issue? Would at least assigning NULL on exit be more safe? (Because yes, there just shouldn't be two APM drivers, and for the weird case there're, it would be nice to at least not segfault.)



-- 
Best regards,
 Paul                            mailto:pmiscml@gmail.com

Re: [PATCH 7/7] [RFC] APM emulation driver for class batteries

[Pavel Machek]

Hi!

> > > +static void (*old_apm_get_power_status)(struct apm_power_info*);
> > > +
> > > +static int __init apm_battery_init(void)
> > > +{
> > > +	printk(KERN_INFO "APM Battery Driver\n");
> > > +
> > > +	old_apm_get_power_status = apm_get_power_status;
> > > +	apm_get_power_status = apm_battery_apm_get_power_status;
> > > +	return 0;
> > > +}
> > > +
> > > +static void __exit apm_battery_exit(void)
> > > +{
> > > +	apm_get_power_status = old_apm_get_power_status;
> > > +	return;
> > > +}
> > 
> > Utterly unsafe.  What happens if some other module gets loaded which
> > does this, and then this module is unloaded followed by the other
> > module.  Result: Oops.
> 
> Right. And loading two modules which changing apm_get_power_status
> is a race already. Thus, APM interface needs a mutex.
> 
> Or pda_power should be marked "bool" in Kconfig, as it is done
> in arch/arm/common/sharpsl_pm.c. Sharpsl_pm is safe only because it
> can't be a module.

Just mark it bool, when you fix it with mutex, you can go back to
tristate.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html