[suspend/resume] Re: userspace notification from module

[suspend/resume] Re: userspace notification from module

[Pavel Machek]

On Sat 2010-01-09 14:40:46, Rafael J. Wysocki wrote:
> On Saturday 09 January 2010, Pavel Machek wrote:

> > > Perhaps I don't understand correctly what you're trying to achieve, but at the
> > > moment suspend is always started from user space, this way or another, and on
> > 
> > At least zaurus (arm) suspends from kernel on battery critical.
> 
> I wasn't aware of this.
> 
> That may be a good reason for adding kernel-based suspend notification,
> although I'd prefer ARM to notify the user space about the critical battery
> status allowing it to decide what to do.

Hard to do, without breaking compatibility that goes down to 2.4.X.

> IMhO automatic suspend without something like the Android's wakelocks hurts
> more than it helps.

It really makes sense on zaurus. Those machines are simple, no
smartbattery and no embedded controller subsystems. Battery will not
protect itself, and its kernel job. (Should work on init=/bin/bash).

As power-off consumption is same as suspend power consumption (I
beleive zaurus simply does not have true power off), suspend on
critical makes some sense. (Note that it is set lower than on pcs, and
that we declare battery critical sooner than that.)
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[suspend/resume] Re: userspace notification from module

[Rafael J. Wysocki]

On Friday 15 January 2010, Pavel Machek wrote:
> On Sat 2010-01-09 14:40:46, Rafael J. Wysocki wrote:
> > On Saturday 09 January 2010, Pavel Machek wrote:
> 
> > > > Perhaps I don't understand correctly what you're trying to achieve, but at the
> > > > moment suspend is always started from user space, this way or another, and on
> > > 
> > > At least zaurus (arm) suspends from kernel on battery critical.
> > 
> > I wasn't aware of this.
> > 
> > That may be a good reason for adding kernel-based suspend notification,
> > although I'd prefer ARM to notify the user space about the critical battery
> > status allowing it to decide what to do.
> 
> Hard to do, without breaking compatibility that goes down to 2.4.X.

Sending a battery-critical notification to the user space is not equivalent to
removing the existing kernel-based mechanism.  They can exist both at the
same time if the notification is sent earlier than the kernel suspends
everything. 

> > IMhO automatic suspend without something like the Android's wakelocks hurts
> > more than it helps.
> 
> It really makes sense on zaurus. Those machines are simple, no
> smartbattery and no embedded controller subsystems. Battery will not
> protect itself, and its kernel job. (Should work on init=/bin/bash).
> 
> As power-off consumption is same as suspend power consumption (I
> beleive zaurus simply does not have true power off), suspend on
> critical makes some sense. (Note that it is set lower than on pcs, and
> that we declare battery critical sooner than that.)

The problem with that is it catches at least some applications unprepared and
notifying them that "we're suspending right now" doesn't really help, because
they won't have any time to react anyway.

Rafael

[suspend/resume] Re: userspace notification from module

[Eric Miao]

On Sat, Jan 16, 2010 at 6:14 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Friday 15 January 2010, Pavel Machek wrote:
>> On Sat 2010-01-09 14:40:46, Rafael J. Wysocki wrote:
>> > On Saturday 09 January 2010, Pavel Machek wrote:
>>
>> > > > Perhaps I don't understand correctly what you're trying to achieve, but at the
>> > > > moment suspend is always started from user space, this way or another, and on
>> > >
>> > > At least zaurus (arm) suspends from kernel on battery critical.
>> >
>> > I wasn't aware of this.
>> >
>> > That may be a good reason for adding kernel-based suspend notification,
>> > although I'd prefer ARM to notify the user space about the critical battery
>> > status allowing it to decide what to do.
>>
>> Hard to do, without breaking compatibility that goes down to 2.4.X.
>
> Sending a battery-critical notification to the user space is not equivalent to
> removing the existing kernel-based mechanism. ?They can exist both at the
> same time if the notification is sent earlier than the kernel suspends
> everything.
>

Pavel,

I'd agree with Rafael that we need to move battery management forward to
a more standard way though we can keep the compatibility atm. The code
of sharpsl-pm.c is no way clean and maintainable. Having a battery driver
instead would be a better way to go in a long run.

And the other way we may need to look into what API the current userland
apps on zaurus is depending on this 2.4 compatibility and make changes
slowly to those apps.

>> > IMhO automatic suspend without something like the Android's wakelocks hurts
>> > more than it helps.
>>
>> It really makes sense on zaurus. Those machines are simple, no
>> smartbattery and no embedded controller subsystems. Battery will not
>> protect itself, and its kernel job. (Should work on init=/bin/bash).
>>
>> As power-off consumption is same as suspend power consumption (I
>> beleive zaurus simply does not have true power off), suspend on
>> critical makes some sense. (Note that it is set lower than on pcs, and
>> that we declare battery critical sooner than that.)
>
> The problem with that is it catches at least some applications unprepared and
> notifying them that "we're suspending right now" doesn't really help, because
> they won't have any time to react anyway.
>
> Rafael
>

[suspend/resume] Re: userspace notification from module

[Stanislav Brabec]

Eric Miao wrote:

> And the other way we may need to look into what API the current userland
> apps on zaurus is depending on this 2.4 compatibility and make changes
> slowly to those apps.

I guess that 2.4 compatibility is not an issue. Most modern Zaurus
distributions are even unable to run Sharp ROM compatible binaries.

Distributions either stay on 2.4 kernel or use modern systems based on
modern kernel 2.6 API.

Distributions that decided to migrate to kernel 2.6 are far from
finished state. Any change that allows to use modern applications using
standard kernel API is welcome.


________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx/zaurus

[suspend/resume] Re: userspace notification from module

[Pavel Machek]

Hi!

> > > I wasn't aware of this.
> > > 
> > > That may be a good reason for adding kernel-based suspend notification,
> > > although I'd prefer ARM to notify the user space about the critical battery
> > > status allowing it to decide what to do.
> > 
> > Hard to do, without breaking compatibility that goes down to 2.4.X.
> 
> Sending a battery-critical notification to the user space is not equivalent to
> removing the existing kernel-based mechanism.  They can exist both at the
> same time if the notification is sent earlier than the kernel suspends
> everything. 

Yes, and obviously sending notification early is ok with me.

> > It really makes sense on zaurus. Those machines are simple, no
> > smartbattery and no embedded controller subsystems. Battery will not
> > protect itself, and its kernel job. (Should work on init=/bin/bash).
> > 
> > As power-off consumption is same as suspend power consumption (I
> > beleive zaurus simply does not have true power off), suspend on
> > critical makes some sense. (Note that it is set lower than on pcs, and
> > that we declare battery critical sooner than that.)
> 
> The problem with that is it catches at least some applications unprepared and
> notifying them that "we're suspending right now" doesn't really help, because
> they won't have any time to react anyway.

Agreed, but so what? On PC, machine would power off at that
point. That would surprise the apps, too.

Basically new enough userland should not make battery run low enough
for either emergency power off or emergency suspend.

IOW "nothing to see here, problem does not exist".
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[suspend/resume] Re: userspace notification from module

[Pavel Machek]

Hi!

> >> > That may be a good reason for adding kernel-based suspend notification,
> >> > although I'd prefer ARM to notify the user space about the critical battery
> >> > status allowing it to decide what to do.
> >>
> >> Hard to do, without breaking compatibility that goes down to 2.4.X.
> >
> > Sending a battery-critical notification to the user space is not equivalent to
> > removing the existing kernel-based mechanism. ?They can exist both at the
> > same time if the notification is sent earlier than the kernel suspends
> > everything.
> 
> I'd agree with Rafael that we need to move battery management forward to
> a more standard way though we can keep the compatibility atm. The code
> of sharpsl-pm.c is no way clean and maintainable. Having a battery driver
> instead would be a better way to go in a long run.

Something like below? Yes, I'd like to get something like that pushed.

> And the other way we may need to look into what API the current userland
> apps on zaurus is depending on this 2.4 compatibility and make changes
> slowly to those apps.

Well, emergency suspend/poweroff should still work in init=/bin/bash
mode. And doing emergency poweroff when we are capable of suspend
seems wrong. 

									Pavel

--- ./drivers/power.ofic/Makefile	2009-10-06 13:51:29.000000000 +0200
+++ ./drivers/power/Makefile	2009-10-11 16:12:09.000000000 +0200
@@ -24,6 +24,7 @@
 obj-$(CONFIG_BATTERY_PMU)	+= pmu_battery.o
 obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
 obj-$(CONFIG_BATTERY_TOSA)	+= tosa_battery.o
+obj-m	+= spitz_battery.o
 obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
 obj-$(CONFIG_BATTERY_BQ27x00)	+= bq27x00_battery.o
 obj-$(CONFIG_BATTERY_DA9030)	+= da9030_battery.o
diff -ur ./drivers/power.ofic/power_supply_sysfs.c ./drivers/power/power_supply_sysfs.c
--- ./drivers/power.ofic/power_supply_sysfs.c	2009-10-06 13:51:29.000000000 +0200
+++ ./drivers/power/power_supply_sysfs.c	2009-10-15 05:45:46.000000000 +0200
@@ -39,7 +39,8 @@
 
 static ssize_t power_supply_show_property(struct device *dev,
 					  struct device_attribute *attr,
-					  char *buf) {
+					  char *buf)
+{
 	static char *status_text[] = {
 		"Unknown", "Charging", "Discharging", "Not charging", "Full"
 	};
@@ -135,7 +136,8 @@
 
 static ssize_t power_supply_show_static_attrs(struct device *dev,
 					      struct device_attribute *attr,
-					      char *buf) {
+					      char *buf)
+{
 	static char *type_text[] = { "Battery", "UPS", "Mains", "USB" };
 	struct power_supply *psy = dev_get_drvdata(dev);
 
diff -ur ./drivers/power.ofic/spitz_battery.c ./drivers/power/spitz_battery.c
--- ./drivers/power.ofic/spitz_battery.c	2009-10-11 16:14:11.000000000 +0200
+++ ./drivers/power/spitz_battery.c	2009-10-22 07:27:52.000000000 +0200
@@ -0,0 +1,430 @@
+/*
+ * Battery and Power Management code for the Sharp SL-3000c
+ *
+ * Copyright (c) 2009 Pavel Machek <pavel@ucw.cz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Li-ion batteries are angry beasts, and they like to explode.
+ * If angry lithium comes your way, the hw was misdesigned.
+ *
+ */
+#include <linux/platform_device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/power_supply.h>
+#include <linux/delay.h>
+#include <linux/spinlock.h>
+#include <linux/interrupt.h>
+#include <linux/gpio.h>
+
+#include <asm/mach-types.h>
+#include <mach/spitz.h>
+#include <mach/sharpsl.h>
+#include <mach/sharpsl_pm.h>
+
+#include "../../arch/arm/mach-pxa/sharpsl.h"
+
+extern struct sharpsl_pm_status sharpsl_pm;
+
+
+struct spitz_bat {
+	struct power_supply psy;
+
+	bool (*is_present)(struct spitz_bat *bat);
+};
+
+static struct spitz_bat spitz_bat_main, spitz_ac;
+
+extern int sharpsl_pm_pxa_read_max1111(int channel);
+
+int basic_current = 125; /* miliAmp */
+int battery_resistance = 422; /* miliOhm */
+
+/* 422 seems to be suitable for very old, 1Ah battery.
+   2Ah battery probably has better resistance */
+
+/* Unfortunately, resistance depends on state of charge, current
+ * direction and temperature.
+ *
+ * Ouch, and dependency is actually _not_ too simple. It is lowest
+ * at 3.55V, very slowly rises at 4V (approximately linear dependency),
+ * and quickly rises towards 3.2V (in something exponential-looking).
+ *
+ * It is about same at 25Celsius and 40Celsius, and about 2.5x the value
+ * on 0Celsius, rising _very_ sharply.
+ *
+ * Li-ion should only be charged between 0 and 45 Celsius, and discharged
+ * between -20 and 60 celsius.
+ */
+
+extern int backlight_current;
+
+int battery_current(void)
+{
+	int intensity = sharpsl_pm.machinfo->backlight_get_status ? sharpsl_pm.machinfo->backlight_get_status() : 0;
+
+	return basic_current + backlight_current;
+}
+
+int liion_internal_voltage(int voltage, int current_ma)
+{
+	return voltage + (battery_resistance * current_ma / 1000);
+}
+
+int liion_expected_voltage(int internal_voltage, int current_ma)
+{       
+	return internal_voltage - (battery_resistance * current_ma / 1000);
+}
+
+/* returns mV */
+int liion_voltage(void)
+{
+	/* Thanks to Stanislav B. ADC has 3.3V as reference,
+	   is connected to battery over 47kOhm,
+	   and to ground over 100kOhm. */
+	return (sharpsl_pm.machinfo->read_devdata(SHARPSL_BATT_VOLT) * 147 * 33)/256;
+}
+
+/* See for example http://www.kokam.com/english/biz/rc.html for
+ * voltage/capacity characteristic. I assume it is going to be
+ * reasonably similar to li-ion used in collie.
+ *
+ */
+
+/*
+ { 420, 100 },
+ { 417, 95 }, means it will report 100% between 418 and 420
+ */
+
+struct battery_thresh battery_levels[] = {
+	{ 3980, 100 },
+	{ 3900, 95 },
+	{ 3860, 90 },
+	{ 3800, 85 },
+	{ 3760, 80 },
+	{ 3720, 74 },
+	{ 3680, 69 },
+	{ 3620, 65 },
+	{ 3570, 59 },
+	{ 3560, 55 },
+	{ 3550, 48 },
+	{ 3530, 45 },
+	{ 3510, 39 },
+	{ 3490, 33 },
+	{ 3470, 29 },
+	{ 3450, 23 },
+	{ 3410, 16 },
+	{    0, 0 },
+};
+
+int get_percentage(void)
+{
+	int i = ARRAY_SIZE(battery_levels);
+	struct battery_thresh *thresh;
+	int voltage = liion_internal_voltage(liion_voltage(), battery_current());
+
+	thresh = battery_levels;
+
+	while (i > 0 && (voltage > thresh[i].voltage))
+		i--;
+
+	return thresh[i].percentage;
+}
+
+static int spitz_bat_get_property(struct power_supply *psy,
+			    enum power_supply_property psp,
+			    union power_supply_propval *val)
+{
+	int ret = 0;
+	struct spitz_bat *bat = container_of(psy, struct spitz_bat, psy);
+
+	val->intval = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_HEALTH:
+		/* POWER_SUPPLY_HEALTH_OVERHEAT , POWER_SUPPLY_HEALTH_COLD,
+		   POWER_SUPPLY_HEALTH_OVERVOLTAGE, POWER_SUPPLY_HEALTH_UNSPEC_FAILURE, POWER_SUPPLY_HEALTH_GOOD
+		 */
+		return 0;
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
+	  	if (gpio_get_value(SPITZ_GPIO_CHRG_ON) == 0) {
+			if (gpio_get_value(SPITZ_GPIO_JK_B) == 1)
+				val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
+			else
+				val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+		}
+		return 0;
+	case POWER_SUPPLY_PROP_STATUS:
+	  {
+		int status = 0;
+
+	  	if (gpio_get_value(SPITZ_GPIO_CHRG_ON) == 0)
+			printk("Chrg bit on. ");
+	  	if (gpio_get_value(SPITZ_GPIO_JK_B) == 0)
+			printk("Slow charge bit on. ");
+
+		val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+
+		if (!sharpsl_pm.machinfo->read_devdata(SHARPSL_STATUS_ACIN))
+			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+		else {
+			if (gpio_get_value(SPITZ_GPIO_CHRG_ON) == 0)
+				val->intval = POWER_SUPPLY_STATUS_CHARGING;
+			if (sharpsl_pm.machinfo->read_devdata(SHARPSL_STATUS_CHRGFULL))
+				val->intval = POWER_SUPPLY_STATUS_FULL;
+		}
+
+		printk("ACIN: %d ", sharpsl_pm.machinfo->read_devdata(SHARPSL_STATUS_ACIN));
+		printk("Chrgfull: %d  ", sharpsl_pm.machinfo->read_devdata(SHARPSL_STATUS_CHRGFULL));
+		printk("Fatal: %d  ", sharpsl_pm.machinfo->read_devdata(SHARPSL_STATUS_FATAL));
+		printk("ACIN_volt: %d\n", sharpsl_pm.machinfo->read_devdata(SHARPSL_ACIN_VOLT));
+
+		return 0;
+	  }
+	case POWER_SUPPLY_PROP_TECHNOLOGY:
+		val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = liion_voltage()*1000;
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		val->intval = 4200000;
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		val->intval = 3400000;
+		return 0;
+	case POWER_SUPPLY_PROP_TEMP:
+		mdelay(SHARPSL_CHECK_BATTERY_WAIT_TIME_TEMP);
+		sharpsl_pm.machinfo->measure_temp(1);
+		mdelay(SHARPSL_CHECK_BATTERY_WAIT_TIME_TEMP);
+		val->intval = sharpsl_pm.machinfo->read_devdata(SHARPSL_BATT_TEMP);
+		sharpsl_pm.machinfo->measure_temp(0);
+		/* 121: battery finished charging in 22C room */
+		/* 141: outside at 6C */
+		return 0;
+	case POWER_SUPPLY_PROP_MODEL_NAME:
+		val->strval = "spitz-battery";
+		return 0;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		return 0;
+		/* add these */
+	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+		val->intval = 2000000;
+		return 0;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		val->intval = get_percentage();
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_AVG:
+		val->intval = liion_internal_voltage(liion_voltage(), battery_current())*1000;
+		return 0;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		val->intval = battery_current() * 1000;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+	return -EINVAL;
+}
+
+static int spitz_ac_get_property(struct power_supply *psy,
+			    enum power_supply_property psp,
+			    union power_supply_propval *val)
+{
+	int ret = 0;
+	struct spitz_bat *bat = container_of(psy, struct spitz_bat, psy);
+
+	val->intval = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		/* Thanks to Stanislav B. ADC has 3.3V as reference,
+		   is connected to acin over 2kOhm,
+		   and to ground over 1kOhm. */
+		val->intval = (sharpsl_pm.machinfo->read_devdata(SHARPSL_ACIN_VOLT) * 3000 * 3300)/256;
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		val->intval = 5250000;
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		val->intval = 4750000;
+		return 0;
+	case POWER_SUPPLY_PROP_MODEL_NAME:
+		val->strval = "spitz-power-supply";
+		return 0;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = sharpsl_pm.machinfo->read_devdata(SHARPSL_STATUS_ACIN);
+		return 0;
+	default:
+		return -EINVAL;
+	}
+	return -EINVAL;
+}
+
+static void spitz_bat_external_power_changed(struct power_supply *psy)
+{
+}
+
+
+static enum power_supply_property spitz_bat_main_props[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_VOLTAGE_AVG,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static struct spitz_bat spitz_bat_main = {
+	.psy = {
+		.name		= "main-battery",
+		.type		= POWER_SUPPLY_TYPE_BATTERY,
+		.properties	= spitz_bat_main_props,
+		.num_properties	= ARRAY_SIZE(spitz_bat_main_props),
+		.get_property	= spitz_bat_get_property,
+		.external_power_changed = spitz_bat_external_power_changed,
+		.use_for_apm	= 1,
+	},
+};
+
+static enum power_supply_property spitz_ac_props[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_PRESENT,
+};
+
+static struct spitz_bat spitz_ac = {
+	.psy = {
+		.name		= "ac",
+		.type		= POWER_SUPPLY_TYPE_MAINS,
+		.properties	= spitz_ac_props,
+		.num_properties	= ARRAY_SIZE(spitz_ac_props),
+		.get_property	= spitz_ac_get_property,
+	},
+};
+
+#ifdef CONFIG_PM
+static int spitz_bat_suspend(struct platform_device *dev, pm_message_t state)
+{
+	return 0;
+}
+
+static int spitz_bat_resume(struct platform_device *dev)
+{
+	return 0;
+}
+#else
+#define spitz_bat_suspend NULL
+#define spitz_bat_resume NULL
+#endif
+
+
+static ssize_t spitz_bat_limit_show(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	sprintf(buf, "Hello :-)");
+	return 9;
+}
+
+static ssize_t spitz_bat_limit_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	if (!strncmp(buf, "fastcharge", 10)) {
+		gpio_set_value(SPITZ_GPIO_JK_B, 1);
+		gpio_set_value(SPITZ_GPIO_CHRG_ON, 0);
+		return 10;
+	}
+	if (!strncmp(buf, "slowcharge", 10)) {
+		gpio_set_value(SPITZ_GPIO_JK_B, 0);
+		gpio_set_value(SPITZ_GPIO_CHRG_ON, 0);
+		return 10;
+	}
+	if (!strncmp(buf, "nonecharge", 10)) {
+		gpio_set_value(SPITZ_GPIO_JK_B, 0);
+		gpio_set_value(SPITZ_GPIO_CHRG_ON, 1);
+		return 10;
+	}
+	return -EINVAL;
+}
+
+
+static DEVICE_ATTR(limit, S_IRUGO | S_IWUSR, spitz_bat_limit_show, spitz_bat_limit_store);
+#if 0
+static struct device_attribute dev_attr_limit = {
+        .attr = { .name = "limit", .mode = 0644 },
+        .show = spitz_bat_limit_show,
+        .store = spitz_bat_limit_store,
+};
+#endif
+
+static int __devinit spitz_bat_probe(struct platform_device *dev)
+{
+	int ret;
+	int i;
+
+	if (!machine_is_spitz())
+		return -ENODEV;
+
+	printk("spitz_bat_probe: register\n");
+	power_supply_register(&dev->dev, &spitz_bat_main.psy);
+	power_supply_register(&dev->dev, &spitz_ac.psy);
+	device_create_file(&dev->dev, &dev_attr_limit);
+
+	return 0;
+}
+
+static int __devexit spitz_bat_remove(struct platform_device *dev)
+{
+	int i;
+
+	device_remove_file(&dev->dev, &dev_attr_limit);
+	power_supply_unregister(&spitz_bat_main.psy);
+	power_supply_unregister(&spitz_ac.psy);
+	return 0;
+}
+
+
+static struct platform_driver spitz_bat_driver = {
+	.driver.name	= "spitz-battery",
+	.driver.owner	= THIS_MODULE,
+	.probe		= spitz_bat_probe,
+	.remove		= __devexit_p(spitz_bat_remove),
+	.suspend	= spitz_bat_suspend,
+	.resume		= spitz_bat_resume,
+};
+
+static int __init spitz_bat_init(void)
+{
+	return platform_driver_register(&spitz_bat_driver);
+}
+
+static void __exit spitz_bat_exit(void)
+{
+	platform_driver_unregister(&spitz_bat_driver);
+}
+
+module_init(spitz_bat_init);
+module_exit(spitz_bat_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Pavel Machek");
+MODULE_DESCRIPTION("Spitz battery driver");
+MODULE_ALIAS("platform:spitz-battery");
diff -ur ./drivers/video/backlight.ofic/corgi_lcd.c ./drivers/video/backlight/corgi_lcd.c
--- ./drivers/video/backlight.ofic/corgi_lcd.c	2009-10-18 18:11:36.000000000 +0200
+++ ./drivers/video/backlight/corgi_lcd.c	2009-10-18 18:37:09.000000000 +0200
@@ -388,17 +388,33 @@
 	.set_mode	= corgi_lcd_set_mode,
 };
 
-static int corgi_bl_get_intensity(struct backlight_device *bd)
+int corgi_bl_get_intensity(struct backlight_device *bd)
 {
 	struct corgi_lcd *lcd = dev_get_drvdata(&bd->dev);
 
 	return lcd->intensity;
 }
+EXPORT_SYMBOL(corgi_bl_get_intensity);
+
+int backlight_current;
+EXPORT_SYMBOL(backlight_current);
 
 static int corgi_bl_set_intensity(struct corgi_lcd *lcd, int intensity)
 {
 	int cont;
 
+	backlight_current = 0;
+	if (intensity > 0)
+		backlight_current = 55;
+	if (intensity > 10)
+		backlight_current = 115;
+	if (intensity > 20)
+		backlight_current = 140;
+	if (intensity > 30)
+		backlight_current = 180;
+	if (intensity > 45)
+		backlight_current = 245;
+
 	if (intensity > 0x10)
 		intensity += 0x10;
 
@@ -433,8 +449,9 @@
 
 	if (corgibl_flags & CORGIBL_SUSPENDED)
 		intensity = 0;
-	if (corgibl_flags & CORGIBL_BATTLOW)
-		intensity &= lcd->limit_mask;
+
+	if ((corgibl_flags & CORGIBL_BATTLOW) && intensity > lcd->limit_mask)
+		intensity = lcd->limit_mask;
 
 	return corgi_bl_set_intensity(lcd, intensity);
 }
diff -ur ./arch/arm.ofic/mach-pxa/corgi_pm.c ./arch/arm/mach-pxa/corgi_pm.c
--- ./arch/arm.ofic/mach-pxa/corgi_pm.c	2009-09-10 00:13:59.000000000 +0200
+++ ./arch/arm/mach-pxa/corgi_pm.c	2009-10-22 19:19:02.000000000 +0200
@@ -35,6 +35,92 @@
 #define SHARPSL_FATAL_ACIN_VOLT        182   /* 3.45V */
 #define SHARPSL_FATAL_NOACIN_VOLT      170   /* 3.40V */
 
+static const struct battery_thresh corgi_battery_levels_acin[] = {
+	{ 213, 100},
+	{ 212,  98},
+	{ 211,  95},
+	{ 210,  93},
+	{ 209,  90},
+	{ 208,  88},
+	{ 207,  85},
+	{ 206,  83},
+	{ 205,  80},
+	{ 204,  78},
+	{ 203,  75},
+	{ 202,  73},
+	{ 201,  70},
+	{ 200,  68},
+	{ 199,  65},
+	{ 198,  63},
+	{ 197,  60},
+	{ 196,  58},
+	{ 195,  55},
+	{ 194,  53},
+	{ 193,  50},
+	{ 192,  48},
+	{ 192,  45},
+	{ 191,  43},
+	{ 191,  40},
+	{ 190,  38},
+	{ 190,  35},
+	{ 189,  33},
+	{ 188,  30},
+	{ 187,  28},
+	{ 186,  25},
+	{ 185,  23},
+	{ 184,  20},
+	{ 183,  18},
+	{ 182,  15},
+	{ 181,  13},
+	{ 180,  10},
+	{ 179,   8},
+	{ 178,   5},
+	{   0,   0},
+};
+
+static const struct battery_thresh  corgi_battery_levels_noac[] = {
+	{ 213, 100},
+	{ 212,  98},
+	{ 211,  95},
+	{ 210,  93},
+	{ 209,  90},
+	{ 208,  88},
+	{ 207,  85},
+	{ 206,  83},
+	{ 205,  80},
+	{ 204,  78},
+	{ 203,  75},
+	{ 202,  73},
+	{ 201,  70},
+	{ 200,  68},
+	{ 199,  65},
+	{ 198,  63},
+	{ 197,  60},
+	{ 196,  58},
+	{ 195,  55},
+	{ 194,  53},
+	{ 193,  50},
+	{ 192,  48},
+	{ 191,  45},
+	{ 190,  43},
+	{ 189,  40},
+	{ 188,  38},
+	{ 187,  35},
+	{ 186,  33},
+	{ 185,  30},
+	{ 184,  28},
+	{ 183,  25},
+	{ 182,  23},
+	{ 181,  20},
+	{ 180,  18},
+	{ 179,  15},
+	{ 178,  13},
+	{ 177,  10},
+	{ 176,   8},
+	{ 175,   5},
+	{   0,   0},
+};
+
 static void corgi_charger_init(void)
 {
 	pxa_gpio_mode(CORGI_GPIO_ADC_TEMP_ON | GPIO_OUT);
@@ -214,8 +300,8 @@
 	.fatal_acin_volt  = SHARPSL_FATAL_ACIN_VOLT,
 	.fatal_noacin_volt= SHARPSL_FATAL_NOACIN_VOLT,
 	.bat_levels       = 40,
-	.bat_levels_noac  = spitz_battery_levels_noac,
-	.bat_levels_acin  = spitz_battery_levels_acin,
+	.bat_levels_noac  = corgi_battery_levels_noac,
+	.bat_levels_acin  = corgi_battery_levels_acin,
 	.status_high_acin = 188,
 	.status_low_acin  = 178,
 	.status_high_noac = 185,
diff -ur ./arch/arm.ofic/mach-pxa/sharpsl.h ./arch/arm/mach-pxa/sharpsl.h
--- ./arch/arm.ofic/mach-pxa/sharpsl.h	2009-09-10 00:13:59.000000000 +0200
+++ ./arch/arm/mach-pxa/sharpsl.h	2009-10-14 12:12:13.000000000 +0200
@@ -42,8 +42,23 @@
 #define MAX1111_BATT_TEMP   2u
 #define MAX1111_ACIN_VOLT   6u
 
-extern struct battery_thresh spitz_battery_levels_acin[];
-extern struct battery_thresh spitz_battery_levels_noac[];
 int sharpsl_pm_pxa_read_max1111(int channel);
 
 
+/*
+ * Constants
+ */
+#define SHARPSL_CHARGE_ON_TIME_INTERVAL        (msecs_to_jiffies(1*60*1000))  /* 1 min */
+#define SHARPSL_CHARGE_FINISH_TIME             (msecs_to_jiffies(10*60*1000)) /* 10 min */
+#define SHARPSL_BATCHK_TIME                    (msecs_to_jiffies(15*1000))    /* 15 sec */
+#define SHARPSL_BATCHK_TIME_SUSPEND            (60*10)                        /* 10 min */
+
+#define SHARPSL_WAIT_CO_TIME                   15  /* 15 sec */
+#define SHARPSL_WAIT_DISCHARGE_ON              100 /* 100 msec */
+#define SHARPSL_CHECK_BATTERY_WAIT_TIME_TEMP   10  /* 10 msec */
+#define SHARPSL_CHECK_BATTERY_WAIT_TIME_VOLT   10  /* 10 msec */
+#define SHARPSL_CHECK_BATTERY_WAIT_TIME_ACIN   10  /* 10 msec */
+#define SHARPSL_CHARGE_WAIT_TIME               15  /* 15 msec */
+#define SHARPSL_CHARGE_CO_CHECK_TIME           5   /* 5 msec */
+#define SHARPSL_CHARGE_RETRY_CNT               1   /* eqv. 10 min */
+
Only in ./arch/arm/mach-pxa: sharpsl.h~
diff -ur ./arch/arm.ofic/mach-pxa/sharpsl_pm.c ./arch/arm/mach-pxa/sharpsl_pm.c
--- ./arch/arm.ofic/mach-pxa/sharpsl_pm.c	2009-10-06 13:48:07.000000000 +0200
+++ ./arch/arm/mach-pxa/sharpsl_pm.c	2009-10-14 12:12:05.000000000 +0200
@@ -32,25 +32,10 @@
 #include <mach/regs-rtc.h>
 #include <mach/sharpsl.h>
 #include <mach/sharpsl_pm.h>
+#include <mach/spitz.h>
 
 #include "sharpsl.h"
 
-/*
- * Constants
- */
-#define SHARPSL_CHARGE_ON_TIME_INTERVAL        (msecs_to_jiffies(1*60*1000))  /* 1 min */
-#define SHARPSL_CHARGE_FINISH_TIME             (msecs_to_jiffies(10*60*1000)) /* 10 min */
-#define SHARPSL_BATCHK_TIME                    (msecs_to_jiffies(15*1000))    /* 15 sec */
-#define SHARPSL_BATCHK_TIME_SUSPEND            (60*10)                        /* 10 min */
-
-#define SHARPSL_WAIT_CO_TIME                   15  /* 15 sec */
-#define SHARPSL_WAIT_DISCHARGE_ON              100 /* 100 msec */
-#define SHARPSL_CHECK_BATTERY_WAIT_TIME_TEMP   10  /* 10 msec */
-#define SHARPSL_CHECK_BATTERY_WAIT_TIME_VOLT   10  /* 10 msec */
-#define SHARPSL_CHECK_BATTERY_WAIT_TIME_ACIN   10  /* 10 msec */
-#define SHARPSL_CHARGE_WAIT_TIME               15  /* 15 msec */
-#define SHARPSL_CHARGE_CO_CHECK_TIME           5   /* 5 msec */
-#define SHARPSL_CHARGE_RETRY_CNT               1   /* eqv. 10 min */
 
 /*
  * Prototypes
@@ -72,112 +57,28 @@
  * Variables
  */
 struct sharpsl_pm_status sharpsl_pm;
+EXPORT_SYMBOL(sharpsl_pm);
 static DECLARE_DELAYED_WORK(toggle_charger, sharpsl_charge_toggle);
 static DECLARE_DELAYED_WORK(sharpsl_bat, sharpsl_battery_thread);
 DEFINE_LED_TRIGGER(sharpsl_charge_led_trigger);
 
 
 
-struct battery_thresh spitz_battery_levels_acin[] = {
-	{ 213, 100},
-	{ 212,  98},
-	{ 211,  95},
-	{ 210,  93},
-	{ 209,  90},
-	{ 208,  88},
-	{ 207,  85},
-	{ 206,  83},
-	{ 205,  80},
-	{ 204,  78},
-	{ 203,  75},
-	{ 202,  73},
-	{ 201,  70},
-	{ 200,  68},
-	{ 199,  65},
-	{ 198,  63},
-	{ 197,  60},
-	{ 196,  58},
-	{ 195,  55},
-	{ 194,  53},
-	{ 193,  50},
-	{ 192,  48},
-	{ 192,  45},
-	{ 191,  43},
-	{ 191,  40},
-	{ 190,  38},
-	{ 190,  35},
-	{ 189,  33},
-	{ 188,  30},
-	{ 187,  28},
-	{ 186,  25},
-	{ 185,  23},
-	{ 184,  20},
-	{ 183,  18},
-	{ 182,  15},
-	{ 181,  13},
-	{ 180,  10},
-	{ 179,   8},
-	{ 178,   5},
-	{   0,   0},
-};
-
-struct battery_thresh  spitz_battery_levels_noac[] = {
-	{ 213, 100},
-	{ 212,  98},
-	{ 211,  95},
-	{ 210,  93},
-	{ 209,  90},
-	{ 208,  88},
-	{ 207,  85},
-	{ 206,  83},
-	{ 205,  80},
-	{ 204,  78},
-	{ 203,  75},
-	{ 202,  73},
-	{ 201,  70},
-	{ 200,  68},
-	{ 199,  65},
-	{ 198,  63},
-	{ 197,  60},
-	{ 196,  58},
-	{ 195,  55},
-	{ 194,  53},
-	{ 193,  50},
-	{ 192,  48},
-	{ 191,  45},
-	{ 190,  43},
-	{ 189,  40},
-	{ 188,  38},
-	{ 187,  35},
-	{ 186,  33},
-	{ 185,  30},
-	{ 184,  28},
-	{ 183,  25},
-	{ 182,  23},
-	{ 181,  20},
-	{ 180,  18},
-	{ 179,  15},
-	{ 178,  13},
-	{ 177,  10},
-	{ 176,   8},
-	{ 175,   5},
-	{   0,   0},
-};
-
 /* MAX1111 Commands */
-#define MAXCTRL_PD0      1u << 0
-#define MAXCTRL_PD1      1u << 1
-#define MAXCTRL_SGL      1u << 2
-#define MAXCTRL_UNI      1u << 3
+#define MAXCTRL_PD0      (1u << 0)
+#define MAXCTRL_PD1      (1u << 1)
+#define MAXCTRL_SGL      (1u << 2)
+#define MAXCTRL_UNI      (1u << 3)
 #define MAXCTRL_SEL_SH   4
-#define MAXCTRL_STR      1u << 7
+#define MAXCTRL_STR      (1u << 7)
 
 /*
  * Read MAX1111 ADC
  */
 int sharpsl_pm_pxa_read_max1111(int channel)
 {
-	if (machine_is_tosa()) // Ugly, better move this function into another module
+	/* Ugly, better move this function into another module */
+	if (machine_is_tosa())
 	    return 0;
 
 #ifdef CONFIG_CORGI_SSP_DEPRECATED
@@ -193,7 +94,7 @@
 #endif
 }
 
-static int get_percentage(int voltage)
+int get_percentage(int voltage)
 {
 	int i = sharpsl_pm.machinfo->bat_levels - 1;
 	int bl_status = sharpsl_pm.machinfo->backlight_get_status ? sharpsl_pm.machinfo->backlight_get_status() : 0;
@@ -209,6 +110,7 @@
 
 	return thresh[i].percentage;
 }
+EXPORT_SYMBOL(get_percentage);
 
 static int get_apm_status(int voltage)
 {
@@ -238,7 +140,7 @@
 
 static void sharpsl_battery_thread(struct work_struct *private_)
 {
-	int voltage, percent, apm_status, i = 0;
+	int voltage, percent, apm_status, i;
 
 	if (!sharpsl_pm.machinfo)
 		return;
@@ -250,15 +152,14 @@
 			&& time_after(jiffies, sharpsl_pm.charge_start_time +  SHARPSL_CHARGE_ON_TIME_INTERVAL))
 		schedule_delayed_work(&toggle_charger, 0);
 
-	while(1) {
+	for (i = 0; i < 5; i++) {
 		voltage = sharpsl_pm.machinfo->read_devdata(SHARPSL_BATT_VOLT);
-
-		if (voltage > 0) break;
-		if (i++ > 5) {
-			voltage = sharpsl_pm.machinfo->bat_levels_noac[0].voltage;
-			dev_warn(sharpsl_pm.dev, "Warning: Cannot read main battery!\n");
+		if (voltage > 0)
 			break;
-		}
+	}
+	if (voltage <= 0) {
+		voltage = sharpsl_pm.machinfo->bat_levels_noac[0].voltage;
+		dev_warn(sharpsl_pm.dev, "Warning: Cannot read main battery!\n");
 	}
 
 	voltage = sharpsl_average_value(voltage);
@@ -266,8 +167,10 @@
 	percent = get_percentage(voltage);
 
 	/* At low battery voltages, the voltage has a tendency to start
-           creeping back up so we try to avoid this here */
-	if ((sharpsl_pm.battstat.ac_status == APM_AC_ONLINE) || (apm_status == APM_BATTERY_STATUS_HIGH) ||  percent <= sharpsl_pm.battstat.mainbat_percent) {
+	   creeping back up so we try to avoid this here */
+	if ((sharpsl_pm.battstat.ac_status == APM_AC_ONLINE)
+	    || (apm_status == APM_BATTERY_STATUS_HIGH)
+	    || percent <= sharpsl_pm.battstat.mainbat_percent) {
 		sharpsl_pm.battstat.mainbat_voltage = voltage;
 		sharpsl_pm.battstat.mainbat_status = apm_status;
 		sharpsl_pm.battstat.mainbat_percent = percent;
@@ -279,8 +182,8 @@
 #ifdef CONFIG_BACKLIGHT_CORGI
 	/* If battery is low. limit backlight intensity to save power. */
 	if ((sharpsl_pm.battstat.ac_status != APM_AC_ONLINE)
-			&& ((sharpsl_pm.battstat.mainbat_status == APM_BATTERY_STATUS_LOW) ||
-			(sharpsl_pm.battstat.mainbat_status == APM_BATTERY_STATUS_CRITICAL))) {
+	    && ((sharpsl_pm.battstat.mainbat_status == APM_BATTERY_STATUS_LOW)
+	    || (sharpsl_pm.battstat.mainbat_status == APM_BATTERY_STATUS_CRITICAL))) {
 		if (!(sharpsl_pm.flags & SHARPSL_BL_LIMIT)) {
 			sharpsl_pm.machinfo->backlight_limit(1);
 			sharpsl_pm.flags |= SHARPSL_BL_LIMIT;
@@ -293,8 +196,8 @@
 
 	/* Suspend if critical battery level */
 	if ((sharpsl_pm.battstat.ac_status != APM_AC_ONLINE)
-			&& (sharpsl_pm.battstat.mainbat_status == APM_BATTERY_STATUS_CRITICAL)
-			&& !(sharpsl_pm.flags & SHARPSL_APM_QUEUED)) {
+	     && (sharpsl_pm.battstat.mainbat_status == APM_BATTERY_STATUS_CRITICAL)
+	     && !(sharpsl_pm.flags & SHARPSL_APM_QUEUED)) {
 		sharpsl_pm.flags |= SHARPSL_APM_QUEUED;
 		dev_err(sharpsl_pm.dev, "Fatal Off\n");
 		apm_queue_event(APM_CRITICAL_SUSPEND);
@@ -339,6 +242,8 @@
 
 static void sharpsl_charge_error(void)
 {
+	dev_warn(sharpsl_pm.dev, "Charger Error\n");
+
 	sharpsl_pm_led(SHARPSL_LED_ERROR);
 	sharpsl_pm.machinfo->charge(0);
 	sharpsl_pm.charge_mode = CHRG_ERROR;
@@ -346,7 +251,7 @@
 
 static void sharpsl_charge_toggle(struct work_struct *private_)
 {
-	dev_dbg(sharpsl_pm.dev, "Toogling Charger at time: %lx\n", jiffies);
+	dev_dbg(sharpsl_pm.dev, "Toggling Charger at time: %lx\n", jiffies);
 
 	if (!sharpsl_pm.machinfo->read_devdata(SHARPSL_STATUS_ACIN)) {
 		sharpsl_charge_off();
@@ -368,7 +273,7 @@
 {
 	int acin = sharpsl_pm.machinfo->read_devdata(SHARPSL_STATUS_ACIN);
 
-	dev_dbg(sharpsl_pm.dev, "AC Status: %d\n",acin);
+	dev_dbg(sharpsl_pm.dev, "AC Status: %d\n", acin);
 
 	sharpsl_average_clear();
 	if (acin && (sharpsl_pm.charge_mode != CHRG_ON))
@@ -472,14 +377,14 @@
 	sharpsl_ad[sharpsl_ad_index] = ad;
 	sharpsl_ad_index++;
 	if (sharpsl_ad_index >= SHARPSL_CNV_VALUE_NUM) {
-		for (i=0; i < (SHARPSL_CNV_VALUE_NUM-1); i++)
+		for (i = 0; i < (SHARPSL_CNV_VALUE_NUM-1); i++)
 			sharpsl_ad[i] = sharpsl_ad[i+1];
 		sharpsl_ad_index = SHARPSL_CNV_VALUE_NUM - 1;
 	}
-	for (i=0; i < sharpsl_ad_index; i++)
+	for (i = 0; i < sharpsl_ad_index; i++)
 		ad_val += sharpsl_ad[i];
 
-	return (ad_val / sharpsl_ad_index);
+	return ad_val / sharpsl_ad_index;
 }
 
 /*
@@ -492,8 +397,8 @@
 
 	/* Find MAX val */
 	temp = val[0];
-	j=0;
-	for (i=1; i<5; i++) {
+	j = 0;
+	for (i = 1; i < 5; i++) {
 		if (temp < val[i]) {
 			temp = val[i];
 			j = i;
@@ -502,21 +407,21 @@
 
 	/* Find MIN val */
 	temp = val[4];
-	k=4;
-	for (i=3; i>=0; i--) {
+	k = 4;
+	for (i = 3; i >= 0; i--) {
 		if (temp > val[i]) {
 			temp = val[i];
 			k = i;
 		}
 	}
 
-	for (i=0; i<5; i++)
-		if (i != j && i != k )
+	for (i = 0; i < 5; i++)
+		if (i != j && i != k)
 			sum += val[i];
 
 	dev_dbg(sharpsl_pm.dev, "Average: %d from values: %d, %d, %d, %d, %d\n", sum/3, val[0], val[1], val[2], val[3], val[4]);
 
-	return (sum/3);
+	return sum/3;
 }
 
 static int sharpsl_check_battery_temp(void)
@@ -524,7 +429,7 @@
 	int val, i, buff[5];
 
 	/* Check battery temperature */
-	for (i=0; i<5; i++) {
+	for (i = 0; i < 5; i++) {
 		mdelay(SHARPSL_CHECK_BATTERY_WAIT_TIME_TEMP);
 		sharpsl_pm.machinfo->measure_temp(1);
 		mdelay(SHARPSL_CHECK_BATTERY_WAIT_TIME_TEMP);
@@ -535,8 +440,10 @@
 	val = get_select_val(buff);
 
 	dev_dbg(sharpsl_pm.dev, "Temperature: %d\n", val);
-	if (val > sharpsl_pm.machinfo->charge_on_temp) {
-		printk(KERN_WARNING "Not charging: temperature out of limits.\n");
+	/* FIXME: this should catch battery read errors, but we should
+	   probably avoid charging in <0C temperatures, too. */
+	if ((val < 0) || (val > sharpsl_pm.machinfo->charge_on_temp)) {
+		dev_warn(sharpsl_pm.dev, "Not charging: temperature %d out of limits.\n", val);
 		return -1;
 	}
 
@@ -557,7 +464,7 @@
 		sharpsl_pm.machinfo->discharge1(1);
 
 	/* Check battery voltage */
-	for (i=0; i<5; i++) {
+	for (i = 0; i < 5; i++) {
 		buff[i] = sharpsl_pm.machinfo->read_devdata(SHARPSL_BATT_VOLT);
 		mdelay(SHARPSL_CHECK_BATTERY_WAIT_TIME_VOLT);
 	}
@@ -581,16 +488,16 @@
 {
 	int temp, i, buff[5];
 
-	for (i=0; i<5; i++) {
+	for (i = 0; i < 5; i++) {
 		buff[i] = sharpsl_pm.machinfo->read_devdata(SHARPSL_ACIN_VOLT);
 		mdelay(SHARPSL_CHECK_BATTERY_WAIT_TIME_ACIN);
 	}
 
 	temp = get_select_val(buff);
-	dev_dbg(sharpsl_pm.dev, "AC Voltage: %d\n",temp);
+	dev_dbg(sharpsl_pm.dev, "AC Voltage: %d\n", temp);
 
 	if ((temp > sharpsl_pm.machinfo->charge_acin_high) || (temp < sharpsl_pm.machinfo->charge_acin_low)) {
-		dev_err(sharpsl_pm.dev, "Error: AC check failed.\n");
+		dev_err(sharpsl_pm.dev, "Error: AC check failed: voltage %d.\n", temp);
 		return -1;
 	}
 
@@ -624,9 +531,9 @@
 
 static void corgi_goto_sleep(unsigned long alarm_time, unsigned int alarm_enable, suspend_state_t state)
 {
-	dev_dbg(sharpsl_pm.dev, "Time is: %08x\n",RCNR);
+	dev_dbg(sharpsl_pm.dev, "Time is: %08x\n", RCNR);
 
-	dev_dbg(sharpsl_pm.dev, "Offline Charge Activate = %d\n",sharpsl_pm.flags & SHARPSL_DO_OFFLINE_CHRG);
+	dev_dbg(sharpsl_pm.dev, "Offline Charge Activate = %d\n", sharpsl_pm.flags & SHARPSL_DO_OFFLINE_CHRG);
 	/* not charging and AC-IN! */
 
 	if ((sharpsl_pm.flags & SHARPSL_DO_OFFLINE_CHRG) && (sharpsl_pm.machinfo->read_devdata(SHARPSL_STATUS_ACIN))) {
@@ -644,12 +551,12 @@
 	if ((sharpsl_pm.charge_mode == CHRG_ON) && ((alarm_enable && ((alarm_time - RCNR) > (SHARPSL_BATCHK_TIME_SUSPEND + 30))) || !alarm_enable)) {
 		RTSR &= RTSR_ALE;
 		RTAR = RCNR + SHARPSL_BATCHK_TIME_SUSPEND;
-		dev_dbg(sharpsl_pm.dev, "Charging alarm at: %08x\n",RTAR);
+		dev_dbg(sharpsl_pm.dev, "Charging alarm at: %08x\n", RTAR);
 		sharpsl_pm.flags |= SHARPSL_ALARM_ACTIVE;
 	} else if (alarm_enable) {
 		RTSR &= RTSR_ALE;
 		RTAR = alarm_time;
-		dev_dbg(sharpsl_pm.dev, "User alarm at: %08x\n",RTAR);
+		dev_dbg(sharpsl_pm.dev, "User alarm at: %08x\n", RTAR);
 	} else {
 		dev_dbg(sharpsl_pm.dev, "No alarms set.\n");
 	}
@@ -658,19 +565,20 @@
 
 	sharpsl_pm.machinfo->postsuspend();
 
-	dev_dbg(sharpsl_pm.dev, "Corgi woken up from suspend: %08x\n",PEDR);
+
+	dev_dbg(sharpsl_pm.dev, "Corgi woken up from suspend: %08x\n", PEDR);
 }
 
 static int corgi_enter_suspend(unsigned long alarm_time, unsigned int alarm_enable, suspend_state_t state)
 {
-	if (!sharpsl_pm.machinfo->should_wakeup(!(sharpsl_pm.flags & SHARPSL_ALARM_ACTIVE) && alarm_enable) )
-	{
+  return 0;
+	if (!sharpsl_pm.machinfo->should_wakeup(!(sharpsl_pm.flags & SHARPSL_ALARM_ACTIVE) && alarm_enable)) {
 		if (!(sharpsl_pm.flags & SHARPSL_ALARM_ACTIVE)) {
 			dev_dbg(sharpsl_pm.dev, "No user triggered wakeup events and not charging. Strange. Suspend.\n");
 			corgi_goto_sleep(alarm_time, alarm_enable, state);
 			return 1;
 		}
-		if(sharpsl_off_charge_battery()) {
+		if (sharpsl_off_charge_battery()) {
 			dev_dbg(sharpsl_pm.dev, "Charging. Suspend...\n");
 			corgi_goto_sleep(alarm_time, alarm_enable, state);
 			return 1;
@@ -697,7 +605,7 @@
 
 	corgi_goto_sleep(alarm_time, alarm_status, state);
 
-	while (corgi_enter_suspend(alarm_time,alarm_status,state))
+	while (corgi_enter_suspend(alarm_time, alarm_status, state))
 		{}
 
 	if (sharpsl_pm.machinfo->earlyresume)
@@ -732,7 +640,7 @@
 		sharpsl_pm.machinfo->discharge1(1);
 
 	/* Check battery : check inserting battery ? */
-	for (i=0; i<5; i++) {
+	for (i = 0; i < 5; i++) {
 		buff[i] = sharpsl_pm.machinfo->read_devdata(SHARPSL_BATT_VOLT);
 		mdelay(SHARPSL_CHECK_BATTERY_WAIT_TIME_VOLT);
 	}
@@ -812,7 +720,7 @@
 		mdelay(SHARPSL_CHARGE_CO_CHECK_TIME);
 
 		time = RCNR;
-		while(1) {
+		while (1) {
 			/* Check if any wakeup event had occurred */
 			if (sharpsl_pm.machinfo->charger_wakeup() != 0)
 				return 0;
@@ -835,9 +743,9 @@
 	mdelay(SHARPSL_CHARGE_CO_CHECK_TIME);
 
 	time = RCNR;
-	while(1) {
+	while (1) {
 		/* Check if any wakeup event had occurred */
-		if (sharpsl_pm.machinfo->charger_wakeup() != 0)
+		if (sharpsl_pm.machinfo->charger_wakeup())
 			return 0;
 		/* Check for timeout */
 		if ((RCNR-time) > SHARPSL_WAIT_CO_TIME) {
@@ -864,12 +772,12 @@
 
 static ssize_t battery_percentage_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%d\n",sharpsl_pm.battstat.mainbat_percent);
+	return sprintf(buf, "%d\n", sharpsl_pm.battstat.mainbat_percent);
 }
 
 static ssize_t battery_voltage_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%d\n",sharpsl_pm.battstat.mainbat_voltage);
+	return sprintf(buf, "%d\n", sharpsl_pm.battstat.mainbat_voltage);
 }
 
 static DEVICE_ATTR(battery_percentage, 0444, battery_percentage_show, NULL);
@@ -943,7 +851,6 @@
 		}
 	}
 
-	if (sharpsl_pm.machinfo->batfull_irq)
-	{
+	if (sharpsl_pm.machinfo->batfull_irq) {
 		/* Register interrupt handler. */
 		if (request_irq(IRQ_GPIO(sharpsl_pm.machinfo->gpio_batfull), sharpsl_chrg_full_isr, IRQF_DISABLED | IRQF_TRIGGER_RISING, "CO", sharpsl_chrg_full_isr)) {
diff -ur ./arch/arm.ofic/mach-pxa/spitz.c ./arch/arm/mach-pxa/spitz.c
--- ./arch/arm.ofic/mach-pxa/spitz.c	2009-10-06 13:48:07.000000000 +0200
+++ ./arch/arm/mach-pxa/spitz.c	2009-10-14 11:01:36.000000000 +0200
@@ -686,12 +733,19 @@
 	.dev.platform_data = &sharpsl_rom_data,
 };
 
+static struct platform_device spitz_battery_device = {
+	.name	= "spitz-battery",
+	.id	= -1,
+};
+
+
 static struct platform_device *devices[] __initdata = {
 	&spitzscoop_device,
 	&spitzkbd_device,
 	&spitzled_device,
 	&sharpsl_nand_device,
 	&sharpsl_rom_device,
+	&spitz_battery_device,
 };
 
 static void spitz_poweroff(void)
--- ./arch/arm.ofic/mach-pxa/spitz_pm.c	2009-09-10 00:13:59.000000000 +0200
+++ ./arch/arm/mach-pxa/spitz_pm.c	2009-10-19 07:28:42.000000000 +0200
@@ -37,6 +37,93 @@
 
 static int spitz_last_ac_status;
 
+static const struct battery_thresh spitz_battery_levels_acin[] = {
+	{ 213, 100},
+	{ 212,  98},
+	{ 211,  95},
+	{ 210,  93},
+	{ 209,  90},
+	{ 208,  88},
+	{ 207,  85},
+	{ 206,  83},
+	{ 205,  80},
+	{ 204,  78},
+	{ 203,  75},
+	{ 202,  73},
+	{ 201,  70},
+	{ 200,  68},
+	{ 199,  65},
+	{ 198,  63},
+	{ 197,  60},
+	{ 196,  58},
+	{ 195,  55},
+	{ 194,  53},
+	{ 193,  50},
+	{ 192,  48},
+	{ 192,  45},
+	{ 191,  43},
+	{ 191,  40},
+	{ 190,  38},
+	{ 190,  35},
+	{ 189,  33},
+	{ 188,  30},
+	{ 187,  28},
+	{ 186,  25},
+	{ 185,  23},
+	{ 184,  20},
+	{ 183,  18},
+	{ 182,  15},
+	{ 181,  13},
+	{ 180,  10},
+	{ 179,   8},
+	{ 178,   5},
+	{   0,   0},
+};
+
+static const struct battery_thresh  spitz_battery_levels_noac[] = {
+	{ 213, 100},
+	{ 212,  98},
+	{ 211,  95},
+	{ 210,  93},
+	{ 209,  90},
+	{ 208,  88},
+	{ 207,  85},
+	{ 206,  83},
+	{ 205,  80},
+	{ 204,  78},
+	{ 203,  75},
+	{ 202,  73},
+	{ 201,  70},
+	{ 200,  68},
+	{ 199,  65},
+	{ 198,  63},
+	{ 197,  60},
+	{ 196,  58},
+	{ 195,  55},
+	{ 194,  53},
+	{ 193,  50},
+	{ 192,  48},
+	{ 191,  45},
+	{ 190,  43},
+	{ 189,  40},
+	{ 188,  38},
+	{ 187,  35},
+	{ 186,  33},
+	{ 185,  30},
+	{ 184,  28},
+	{ 183,  25},
+	{ 182,  23},
+	{ 181,  20},
+	{ 180,  18},
+	{ 179,  15},
+	{ 178,  13},
+	{ 177,  10},
+	{ 176,   8},
+	{ 175,   5},
+	{   0,   0},
+};
+
+
 static void spitz_charger_init(void)
 {
 	pxa_gpio_mode(SPITZ_GPIO_KEY_INT | GPIO_IN);
@@ -103,7 +190,7 @@
 	PFER = GPIO_bit(SPITZ_GPIO_KEY_INT) | GPIO_bit(SPITZ_GPIO_RESET);
 	PWER = GPIO_bit(SPITZ_GPIO_KEY_INT) | GPIO_bit(SPITZ_GPIO_RESET) | PWER_RTC;
 	PKWR = GPIO_bit(SPITZ_GPIO_SYNC) | GPIO_bit(SPITZ_GPIO_KEY_INT) | GPIO_bit(SPITZ_GPIO_RESET);
-	PKSR = 0xffffffff; // clear
+	PKSR = 0xffffffff; /* clear */
 
 	/* nRESET_OUT Disable */
 	PSLR |= PSLR_SL_ROD;
@@ -149,7 +236,7 @@
 	if (resume_on_alarm && (PEDR & PWER_RTC))
 		is_resume |= PWER_RTC;
 
-	dev_dbg(sharpsl_pm.dev, "is_resume: %x\n",is_resume);
+	dev_dbg(sharpsl_pm.dev, "is_resume: %x\n", is_resume);
 	return is_resume;
 }
 
@@ -160,15 +247,15 @@
 
 unsigned long spitzpm_read_devdata(int type)
 {
-	switch(type) {
+	switch (type) {
 	case SHARPSL_STATUS_ACIN:
 		return (((~GPLR(SPITZ_GPIO_AC_IN)) & GPIO_bit(SPITZ_GPIO_AC_IN)) != 0);
 	case SHARPSL_STATUS_LOCK:
-		return READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_batlock);
+		return !!READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_batlock);
 	case SHARPSL_STATUS_CHRGFULL:
-		return READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_batfull);
+		return !!READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_batfull);
 	case SHARPSL_STATUS_FATAL:
-		return READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_fatal);
+		return !!READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_fatal);
 	case SHARPSL_ACIN_VOLT:
 		return sharpsl_pm_pxa_read_max1111(MAX1111_ACIN_VOLT);
 	case SHARPSL_BATT_TEMP:
@@ -179,6 +266,11 @@
 	}
 }
 
+int backlight_get_status(void)
+{
+	return 0;
+}
+
 struct sharpsl_charger_machinfo spitz_pm_machinfo = {
 	.init             = spitz_charger_init,
 	.exit             = NULL,
@@ -199,8 +291,9 @@
 #if defined(CONFIG_LCD_CORGI)
 	.backlight_limit = corgi_lcd_limit_intensity,
 #elif defined(CONFIG_BACKLIGHT_CORGI)
-        .backlight_limit  = corgibl_limit_intensity,
+	.backlight_limit  = corgibl_limit_intensity,
 #endif
+//	.backlight_get_status = backlight_get_status,
 	.charge_on_volt	  = SHARPSL_CHARGE_ON_VOLT,
 	.charge_on_temp	  = SHARPSL_CHARGE_ON_TEMP,
 	.charge_acin_high = SHARPSL_CHARGE_ON_ACIN_HIGH,
@@ -241,7 +334,7 @@
 
 static void spitzpm_exit(void)
 {
- 	platform_device_unregister(spitzpm_device);
+	platform_device_unregister(spitzpm_device);
 }
 
 module_init(spitzpm_init);

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

[suspend/resume] Re: userspace notification from module

[Pavel Machek]

On Sat 2010-01-16 18:00:58, Stanislav Brabec wrote:
> Eric Miao wrote:
> 
> > And the other way we may need to look into what API the current userland
> > apps on zaurus is depending on this 2.4 compatibility and make changes
> > slowly to those apps.
> 
> I guess that 2.4 compatibility is not an issue. Most modern Zaurus
> distributions are even unable to run Sharp ROM compatible binaries.
> 
> Distributions either stay on 2.4 kernel or use modern systems based on
> modern kernel 2.6 API.
> 
> Distributions that decided to migrate to kernel 2.6 are far from
> finished state. Any change that allows to use modern applications using
> standard kernel API is welcome.

There is no API involved. It is just ... if you leave zaurus in
 init=/bin/bash mode, it must not kill the battery. Smart and
 currently implemented way to do that is to suspend.

That's counterexample to rjw, but it does not matter -- reasonable
userland should never ever hit that, in a same way PCs should not hit
emergency power cut...
									Pavel

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

[suspend/resume] Re: userspace notification from module

[Rafael J. Wysocki]

On Saturday 16 January 2010, Pavel Machek wrote:
> On Sat 2010-01-16 18:00:58, Stanislav Brabec wrote:
> > Eric Miao wrote:
> > 
> > > And the other way we may need to look into what API the current userland
> > > apps on zaurus is depending on this 2.4 compatibility and make changes
> > > slowly to those apps.
> > 
> > I guess that 2.4 compatibility is not an issue. Most modern Zaurus
> > distributions are even unable to run Sharp ROM compatible binaries.
> > 
> > Distributions either stay on 2.4 kernel or use modern systems based on
> > modern kernel 2.6 API.
> > 
> > Distributions that decided to migrate to kernel 2.6 are far from
> > finished state. Any change that allows to use modern applications using
> > standard kernel API is welcome.
> 
> There is no API involved. It is just ... if you leave zaurus in
>  init=/bin/bash mode, it must not kill the battery. Smart and
>  currently implemented way to do that is to suspend.

IMHO it should just plain shutdown in that case.  Suspending doesn't really
solve the problem, because the battery is going to drain still.  Unless you
mean suspend=hibernate, but I guess you don't.

> That's counterexample to rjw, but it does not matter -- reasonable
> userland should never ever hit that, in a same way PCs should not hit
> emergency power cut...

I don't really understand what you mean.  The user space doesn't know the
battery state if the kernel doesn't tell it, AFAICS, so how exactly can it
predict the critical battery condition without the kernel notifying it?

Rafael

[suspend/resume] Re: userspace notification from module

[Rafael J. Wysocki]

On Saturday 16 January 2010, Pavel Machek wrote:
> Hi!
> 
> > > > I wasn't aware of this.
> > > > 
> > > > That may be a good reason for adding kernel-based suspend notification,
> > > > although I'd prefer ARM to notify the user space about the critical battery
> > > > status allowing it to decide what to do.
> > > 
> > > Hard to do, without breaking compatibility that goes down to 2.4.X.
> > 
> > Sending a battery-critical notification to the user space is not equivalent to
> > removing the existing kernel-based mechanism.  They can exist both at the
> > same time if the notification is sent earlier than the kernel suspends
> > everything. 
> 
> Yes, and obviously sending notification early is ok with me.
> 
> > > It really makes sense on zaurus. Those machines are simple, no
> > > smartbattery and no embedded controller subsystems. Battery will not
> > > protect itself, and its kernel job. (Should work on init=/bin/bash).
> > > 
> > > As power-off consumption is same as suspend power consumption (I
> > > beleive zaurus simply does not have true power off), suspend on
> > > critical makes some sense. (Note that it is set lower than on pcs, and
> > > that we declare battery critical sooner than that.)
> > 
> > The problem with that is it catches at least some applications unprepared and
> > notifying them that "we're suspending right now" doesn't really help, because
> > they won't have any time to react anyway.
> 
> Agreed, but so what? On PC, machine would power off at that
> point. That would surprise the apps, too.
> 
> Basically new enough userland should not make battery run low enough
> for either emergency power off or emergency suspend.

I wonder how it is supposed to achieve that without knowing the current battery
status.  Do you mean it should poll the battery driver?

Rafael

[suspend/resume] Re: userspace notification from module

[Pavel Machek]

Hi!

> > Agreed, but so what? On PC, machine would power off at that
> > point. That would surprise the apps, too.
> > 
> > Basically new enough userland should not make battery run low enough
> > for either emergency power off or emergency suspend.
> 
> I wonder how it is supposed to achieve that without knowing the current battery
> status.  Do you mean it should poll the battery driver?

That's besides point, is it?

Usual kernel<->user interfaces should apply here. Zaurus provides
/proc/apm emulation, and I'm writing drivers/power/ driver. Not sure
it has event interface...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[suspend/resume] Re: userspace notification from module

[Pavel Machek]

On Sat 2010-01-16 23:05:56, Rafael J. Wysocki wrote:
> On Saturday 16 January 2010, Pavel Machek wrote:
> > On Sat 2010-01-16 18:00:58, Stanislav Brabec wrote:
> > > Eric Miao wrote:
> > > 
> > > > And the other way we may need to look into what API the current userland
> > > > apps on zaurus is depending on this 2.4 compatibility and make changes
> > > > slowly to those apps.
> > > 
> > > I guess that 2.4 compatibility is not an issue. Most modern Zaurus
> > > distributions are even unable to run Sharp ROM compatible binaries.
> > > 
> > > Distributions either stay on 2.4 kernel or use modern systems based on
> > > modern kernel 2.6 API.
> > > 
> > > Distributions that decided to migrate to kernel 2.6 are far from
> > > finished state. Any change that allows to use modern applications using
> > > standard kernel API is welcome.
> > 
> > There is no API involved. It is just ... if you leave zaurus in
> >  init=/bin/bash mode, it must not kill the battery. Smart and
> >  currently implemented way to do that is to suspend.
> 
> IMHO it should just plain shutdown in that case.  Suspending doesn't really
> solve the problem, because the battery is going to drain still.  Unless you
> mean suspend=hibernate, but I guess you don't.

As I explained before, power consumption on suspend and hibernate and
poweroff is equivalent on zaurus (7mA in all the cases -- sorry if I
said uA before). And because it has 1800mAh battery, it means that
even empty battery is going to last for a while. In practice, it works
very well.

(There are other reasons, having to do with internal li-ion resistances
in aged and cold batteries.)

> > That's counterexample to rjw, but it does not matter -- reasonable
> > userland should never ever hit that, in a same way PCs should not hit
> > emergency power cut...
> 
> I don't really understand what you mean.  The user space doesn't know the
> battery state if the kernel doesn't tell it, AFAICS, so how exactly can it
> predict the critical battery condition without the kernel notifying it?

That was not the point I was trying to discuss. Yes, we need
kernel<->user notification of battery critical. 

But on zaurus, correct action is to suspend (not hibernate and not
poweroff) when battery is no longer able to supply enough power to
keep system alive.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[suspend/resume] Re: userspace notification from module

[Rafael J. Wysocki]

On Saturday 16 January 2010, Pavel Machek wrote:
> Hi!
> 
> > > Agreed, but so what? On PC, machine would power off at that
> > > point. That would surprise the apps, too.
> > > 
> > > Basically new enough userland should not make battery run low enough
> > > for either emergency power off or emergency suspend.
> > 
> > I wonder how it is supposed to achieve that without knowing the current battery
> > status.  Do you mean it should poll the battery driver?
> 
> That's besides point, is it?

Well, mentioned that first. :-)

> Usual kernel<->user interfaces should apply here.

And we're talking about the design of these, aren't we?  The notification about
the critical battery status sent to the user space is a part of the kernel-user
space interface too, after all.

Rafael

[suspend/resume] Re: userspace notification from module

[Pavel Machek]

> > IMHO it should just plain shutdown in that case.  Suspending doesn't really
> > solve the problem, because the battery is going to drain still.  Unless you
> > mean suspend=hibernate, but I guess you don't.

BTW I was talking spitz before, but there's collie, too. It was
shipped in configuration where all user data was in RAM and RAMdisk --
no writable flash. On such machine, shutdown is never option. (And
yes, such design was very common in windowsCE days).

Spitz got disk, but inherited that powermanagement design; and it
works very well.

And now, if you want collie for a year (or two), you can have
mine... it still works and battery still lasted for 5 hours 2 years
ago -- not bad for 10 year old battery. You'll note that it is very
different design from PC.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[suspend/resume] Re: userspace notification from module

[Rafael J. Wysocki]

On Saturday 16 January 2010, Pavel Machek wrote:
> On Sat 2010-01-16 23:05:56, Rafael J. Wysocki wrote:
> > On Saturday 16 January 2010, Pavel Machek wrote:
> > > On Sat 2010-01-16 18:00:58, Stanislav Brabec wrote:
> > > > Eric Miao wrote:
> > > > 
> > > > > And the other way we may need to look into what API the current userland
> > > > > apps on zaurus is depending on this 2.4 compatibility and make changes
> > > > > slowly to those apps.
> > > > 
> > > > I guess that 2.4 compatibility is not an issue. Most modern Zaurus
> > > > distributions are even unable to run Sharp ROM compatible binaries.
> > > > 
> > > > Distributions either stay on 2.4 kernel or use modern systems based on
> > > > modern kernel 2.6 API.
> > > > 
> > > > Distributions that decided to migrate to kernel 2.6 are far from
> > > > finished state. Any change that allows to use modern applications using
> > > > standard kernel API is welcome.
> > > 
> > > There is no API involved. It is just ... if you leave zaurus in
> > >  init=/bin/bash mode, it must not kill the battery. Smart and
> > >  currently implemented way to do that is to suspend.
> > 
> > IMHO it should just plain shutdown in that case.  Suspending doesn't really
> > solve the problem, because the battery is going to drain still.  Unless you
> > mean suspend=hibernate, but I guess you don't.
> 
> As I explained before, power consumption on suspend and hibernate and
> poweroff is equivalent on zaurus (7mA in all the cases -- sorry if I
> said uA before). And because it has 1800mAh battery, it means that
> even empty battery is going to last for a while. In practice, it works
> very well.
> 
> (There are other reasons, having to do with internal li-ion resistances
> in aged and cold batteries.)
> 
> > > That's counterexample to rjw, but it does not matter -- reasonable
> > > userland should never ever hit that, in a same way PCs should not hit
> > > emergency power cut...
> > 
> > I don't really understand what you mean.  The user space doesn't know the
> > battery state if the kernel doesn't tell it, AFAICS, so how exactly can it
> > predict the critical battery condition without the kernel notifying it?
> 
> That was not the point I was trying to discuss. Yes, we need
> kernel<->user notification of battery critical. 
> 
> But on zaurus, correct action is to suspend (not hibernate and not
> poweroff) when battery is no longer able to supply enough power to
> keep system alive.

Why not to poweroff (just asking, I don't know that hardware)?

I guess we should at least do our best to keep filesystems in a consistent
state and suspend doesn't really guarantee this if the system remains on
battery power afterwards.

Rafael

[suspend/resume] Re: userspace notification from module

[Rafael J. Wysocki]

On Saturday 16 January 2010, Pavel Machek wrote:
> > > IMHO it should just plain shutdown in that case.  Suspending doesn't really
> > > solve the problem, because the battery is going to drain still.  Unless you
> > > mean suspend=hibernate, but I guess you don't.
> 
> BTW I was talking spitz before, but there's collie, too. It was
> shipped in configuration where all user data was in RAM and RAMdisk --
> no writable flash. On such machine, shutdown is never option. (And
> yes, such design was very common in windowsCE days).
> 
> Spitz got disk, but inherited that powermanagement design; and it
> works very well.
> 
> And now, if you want collie for a year (or two), you can have
> mine... it still works and battery still lasted for 5 hours 2 years
> ago -- not bad for 10 year old battery. You'll note that it is very
> different design from PC.

Well, I guess I wouldn't have the time to study it anyway, so thanks. :-)

All in all, it looks like these particular platforms are just specific design
having special requirements.  And even on these platforms sending a battery
critical notification to the user space before the kernel emergency suspends
(or shuts down or whatever) the system seems to be a good idea in general.

Rafael

[suspend/resume] Re: userspace notification from module

[Russell King - ARM Linux]

On Sat, Jan 16, 2010 at 11:05:56PM +0100, Rafael J. Wysocki wrote:
> On Saturday 16 January 2010, Pavel Machek wrote:
> > On Sat 2010-01-16 18:00:58, Stanislav Brabec wrote:
> > > Eric Miao wrote:
> > > 
> > > > And the other way we may need to look into what API the current userland
> > > > apps on zaurus is depending on this 2.4 compatibility and make changes
> > > > slowly to those apps.
> > > 
> > > I guess that 2.4 compatibility is not an issue. Most modern Zaurus
> > > distributions are even unable to run Sharp ROM compatible binaries.
> > > 
> > > Distributions either stay on 2.4 kernel or use modern systems based on
> > > modern kernel 2.6 API.
> > > 
> > > Distributions that decided to migrate to kernel 2.6 are far from
> > > finished state. Any change that allows to use modern applications using
> > > standard kernel API is welcome.
> > 
> > There is no API involved. It is just ... if you leave zaurus in
> >  init=/bin/bash mode, it must not kill the battery. Smart and
> >  currently implemented way to do that is to suspend.
> 
> IMHO it should just plain shutdown in that case.  Suspending doesn't really
> solve the problem, because the battery is going to drain still.  Unless you
> mean suspend=hibernate, but I guess you don't.

FYI, most handheld/palm-based devices don't have a "power off" mode,
short of opening them up and disconnecting the battery.  The "on/off"
button is really just a "suspend/resume" button.

So, when the battery gets critically low, your only option is to suspend.
If the battery continues to be drained, and it's a properly manufacturered
li-ion battery, it will have a discharge protection circuit inside the
battery which will effectively disconnect the battery until it's recharged.
There is no warning of this event.

The whole idea here is in a low battery sitution, is to place the system
into as low power state as possible to try to retain the users data.

(Some protection circuits monitor each li-ion cell, which means if you
have a single weak cell, it can trip before you get the critical suspend
notification.)

There are also some SoCs which have a battery fault input, which forces a
transition to suspend mode with no software intervention what so ever,
placing the SDRAM into self-refresh (data retention) mode.  Wakeup from
such a mode can only be done by total SoC reset once the fault input has
been removed.

Either way, the issue here is that when you get a critical low battery
notification, there may be seconds of power left if the system is fully
operational, and you must suspend as quickly as possible before the
hardware does something more destructive to your state.

Probably the best way of thinking about this is a layered set of
notifications:

- monitor battery level, cleanly suspend when down to N%
- if ignored, you get a critical low battery notification which forces
  an unclean suspend transition, retaining most device, CPU and RAM state.
- if ignored, a hardware initiated suspend transition occurs, retaining
  just RAM state.
- if that doesn't work, the battery itself cuts power and all state is
  lost.

[suspend/resume] Re: userspace notification from module

[Pavel Machek]

> > That was not the point I was trying to discuss. Yes, we need
> > kernel<->user notification of battery critical. 
> > 
> > But on zaurus, correct action is to suspend (not hibernate and not
> > poweroff) when battery is no longer able to supply enough power to
> > keep system alive.
> 
> Why not to poweroff (just asking, I don't know that hardware)?

Those machines basically have no poweroff.

> I guess we should at least do our best to keep filesystems in a consistent
> state and suspend doesn't really guarantee this if the system remains on
> battery power afterwards.

I'm not sure if I ever had battery run so low that it could not keep
RAM running. Collie has even separate (small) battery just for
RAM. (And it also has all the filesystems in ramdisk -- you really do
not want to power it off, even if it could.)

AFAICT following message would be nice.

1) battery is critical, userspace please do something

On zaurus and similar, you could add

2) oh and btw we had power failure so we suspended (or maybe -- so
hardware suspended itself -- rmk's examples and old apm systems); we
are now back and running

notification... but... ideally those power failures should never
happen anyway, so... having this notification is in no way neccessary.

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

[suspend/resume] Re: userspace notification from module

[Russell King - ARM Linux]

On Sun, Jan 17, 2010 at 02:07:39PM +0100, Pavel Machek wrote:
> AFAICT following message would be nice.
> 
> 1) battery is critical, userspace please do something
> 
> On zaurus and similar, you could add
> 
> 2) oh and btw we had power failure so we suspended (or maybe -- so
> hardware suspended itself -- rmk's examples and old apm systems); we
> are now back and running
> 
> notification... but... ideally those power failures should never
> happen anyway, so... having this notification is in no way neccessary.

There's another consideration here: the more complex the emergency
procedure, the higher the chance of _something_ causing it to fail,
and if it does fail, the result is data loss.

In a properly running system, this isn't something that's going to
get a lot of testing, so there's a higher chance that there will be
bugs, so the simpler the solution, the better.

It's a bit like the kernel shutdown paths - because they don't get a
lot of use, they don't get tested enough, and having discussed it with
Arjan van de Ven, it's a known weakness.  So we know that they're not
that well tested - and the result is eg, 33-rc3 on shutdown results
in an oops for me on x86.

You really don't want to oops or deadlock on "battery critically low".

[suspend/resume] Re: userspace notification from module

[Pavel Machek]

On Sun 2010-01-17 13:26:18, Russell King - ARM Linux wrote:
> On Sun, Jan 17, 2010 at 02:07:39PM +0100, Pavel Machek wrote:
> > AFAICT following message would be nice.
> > 
> > 1) battery is critical, userspace please do something
> > 
> > On zaurus and similar, you could add
> > 
> > 2) oh and btw we had power failure so we suspended (or maybe -- so
> > hardware suspended itself -- rmk's examples and old apm systems); we
> > are now back and running
> > 
> > notification... but... ideally those power failures should never
> > happen anyway, so... having this notification is in no way neccessary.
> 
> There's another consideration here: the more complex the emergency
> procedure, the higher the chance of _something_ causing it to fail,
> and if it does fail, the result is data loss.

That's why I propose to only send notification after we resume from
emergency suspend :-).

> In a properly running system, this isn't something that's going to
> get a lot of testing, so there's a higher chance that there will be
> bugs, so the simpler the solution, the better.

Unfortunately, with old battery in zaurus, I tested it rather a lot. I
have new one now, but I can still use old one for testing.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html