[PATCH 0/3] s3c24xx: iPAQ rx1950 series

[PATCH 0/3] s3c24xx: iPAQ rx1950 series

[Vasily Khoruzhick]

This patch series adds more support for iPAQ rx1950 PDA to linux:
1. LEDs driver -- it controls blue, green and red LEDs on rx1950
2. Battery driver -- adds ability to monitor and charge battery.
This driver is suitable for H1940 PDA aswell (just need to write
some machine specific callbacks and get voltage LUTs)

[PATCH 1/3] rx1950: add rx1950 LEDs driver

[Vasily Khoruzhick]

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-s3c2440/mach-rx1950.c |    6 +
 drivers/leds/Kconfig                |    6 +
 drivers/leds/Makefile               |    1 +
 drivers/leds/leds-rx1950.c          |  206 +++++++++++++++++++++++++++++++++++
 4 files changed, 219 insertions(+), 0 deletions(-)
 create mode 100644 drivers/leds/leds-rx1950.c

diff --git a/arch/arm/mach-s3c2440/mach-rx1950.c b/arch/arm/mach-s3c2440/mach-rx1950.c
index 8603b57..55b806a 100644
--- a/arch/arm/mach-s3c2440/mach-rx1950.c
+++ b/arch/arm/mach-s3c2440/mach-rx1950.c
@@ -126,6 +126,11 @@ static struct s3c2410fb_display rx1950_display = {
 
 };
 
+static struct platform_device rx1950_device_leds = {
+	.name             = "rx1950-leds",
+	.id               = -1,
+};
+
 static struct s3c2410fb_mach_info rx1950_lcd_cfg = {
 	.displays = &rx1950_display,
 	.num_displays = 1,
@@ -502,6 +507,7 @@ static struct platform_device *rx1950_devices[] __initdata = {
 	&s3c_device_timer[1],
 	&rx1950_backlight,
 	&rx1950_device_gpiokeys,
+	&rx1950_device_leds,
 };
 
 static struct clk *rx1950_clocks[] __initdata = {
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 81bf25e..f986825 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -102,6 +102,12 @@ config LEDS_H1940
 	help
 	  This option enables support for the LEDs on the h1940.
 
+config LEDS_RX1950
+	tristate "LED Support for iPAQ RX1950 device"
+	depends on MACH_RX1950
+	help
+	  This option enables support for the LEDs on the rx1950.
+
 config LEDS_COBALT_QUBE
 	tristate "LED Support for the Cobalt Qube series front LED"
 	depends on MIPS_COBALT
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 2493de4..faa0d5d 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_LEDS_NET5501)		+= leds-net5501.o
 obj-$(CONFIG_LEDS_WRAP)			+= leds-wrap.o
 obj-$(CONFIG_LEDS_ALIX2)		+= leds-alix2.o
 obj-$(CONFIG_LEDS_H1940)		+= leds-h1940.o
+obj-$(CONFIG_LEDS_RX1950)		+= leds-rx1950.o
 obj-$(CONFIG_LEDS_COBALT_QUBE)		+= leds-cobalt-qube.o
 obj-$(CONFIG_LEDS_COBALT_RAQ)		+= leds-cobalt-raq.o
 obj-$(CONFIG_LEDS_SUNFIRE)		+= leds-sunfire.o
diff --git a/drivers/leds/leds-rx1950.c b/drivers/leds/leds-rx1950.c
new file mode 100644
index 0000000..1eed583
--- /dev/null
+++ b/drivers/leds/leds-rx1950.c
@@ -0,0 +1,206 @@
+/*
+ * drivers/leds/leds-rx1950.c
+ *
+ * Based on leds-h1940 by Arnaud Patard <arnaud.patard@rtp-net.org>
+ * Copyright (c) Vasily Khoruzhick <anarsoul@gmail.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file COPYING in the main directory of this archive for
+ * more details.
+ *
+ * RX1950 leds driver
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/string.h>
+#include <linux/ctype.h>
+#include <linux/leds.h>
+#include <linux/gpio.h>
+
+#include <mach/regs-gpio.h>
+#include <mach/regs-gpioj.h>
+#include <mach/hardware.h>
+
+/*
+ * Green led.
+ */
+static void rx1950_greenled_set(struct led_classdev *led_dev,
+			       enum led_brightness value)
+{
+	switch (value) {
+	case LED_HALF:
+		gpio_direction_output(S3C2410_GPA(4), 1);
+		gpio_direction_output(S3C2410_GPA(6), 0);
+		gpio_direction_output(S3C2410_GPJ(6), 1);
+		break;
+	case LED_FULL:
+		gpio_direction_output(S3C2410_GPA(4), 0);
+		gpio_direction_output(S3C2410_GPA(6), 1);
+		gpio_direction_output(S3C2410_GPJ(6), 0);
+		break;
+	default:
+	case LED_OFF:
+		gpio_direction_output(S3C2410_GPA(4), 0);
+		gpio_direction_output(S3C2410_GPA(6), 0);
+		gpio_direction_output(S3C2410_GPJ(6), 0);
+		break;
+	}
+}
+
+static struct led_classdev rx1950_greenled = {
+	.name			= "rx1950:green",
+	.brightness_set		= rx1950_greenled_set,
+	.default_trigger	= "s3c-adc-charger",
+};
+
+static void rx1950_redled_set(struct led_classdev *led_dev,
+			     enum led_brightness value)
+{
+	switch (value) {
+	case LED_HALF:
+		gpio_direction_output(S3C2410_GPA(3), 1);
+		gpio_direction_output(S3C2410_GPA(7), 0);
+		gpio_direction_output(S3C2410_GPJ(6), 1);
+		break;
+	case LED_FULL:
+		gpio_direction_output(S3C2410_GPA(3), 0);
+		gpio_direction_output(S3C2410_GPA(7), 1);
+		gpio_direction_output(S3C2410_GPJ(6), 0);
+		break;
+	default:
+	case LED_OFF:
+		gpio_direction_output(S3C2410_GPA(3), 0);
+		gpio_direction_output(S3C2410_GPA(7), 0);
+		gpio_direction_output(S3C2410_GPJ(6), 0);
+		break;
+	}
+}
+
+static struct led_classdev rx1950_redled = {
+	.name			= "rx1950:red",
+	.brightness_set		= rx1950_redled_set,
+	.default_trigger	= "s3c-adc-charger",
+};
+
+/*
+ * Blue led.
+ */
+static void rx1950_blueled_set(struct led_classdev *led_dev,
+			      enum led_brightness value)
+{
+	if (value)
+		gpio_direction_output(S3C2410_GPA(11), 1);
+	else
+		gpio_direction_output(S3C2410_GPA(11), 0);
+}
+
+static struct led_classdev rx1950_blueled = {
+	.name			= "rx1950:blue",
+	.brightness_set		= rx1950_blueled_set,
+	.default_trigger	= "rx1950-acx-mem",
+};
+
+static int __devinit rx1950leds_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	ret = gpio_request(S3C2410_GPA(3), "rx1950:red_led_blink");
+	if (ret)
+		goto err_blink_red;
+
+	ret = gpio_request(S3C2410_GPA(4), "rx1950:green_led_blink");
+	if (ret)
+		goto err_blink_green;
+
+	ret = gpio_request(S3C2410_GPJ(6), "rx1950:led_blink");
+	if (ret)
+		goto err_blink;
+
+	ret = gpio_request(S3C2410_GPA(6), "rx1950:green_led");
+	if (ret)
+		goto err_green;
+	ret = led_classdev_register(&pdev->dev, &rx1950_greenled);
+	if (ret)
+		goto err_green_gpio;
+
+	ret = gpio_request(S3C2410_GPA(7), "rx1950:red_led");
+	if (ret)
+		goto err_red;
+	ret = led_classdev_register(&pdev->dev, &rx1950_redled);
+	if (ret)
+		goto err_red_gpio;
+
+	ret = gpio_request(S3C2410_GPA(11), "rx1950:blue_led");
+	if (ret)
+		goto err_blue;
+	ret = led_classdev_register(&pdev->dev, &rx1950_blueled);
+	if (ret)
+		goto err_blue_gpio;
+
+	return 0;
+
+err_blue_gpio:
+	gpio_free(S3C2410_GPA(11));
+err_blue:
+	led_classdev_unregister(&rx1950_redled);
+err_red_gpio:
+	gpio_free(S3C2410_GPA(6));
+err_red:
+	led_classdev_unregister(&rx1950_greenled);
+err_green_gpio:
+	gpio_free(S3C2410_GPA7);
+err_green:
+	gpio_free(S3C2410_GPJ(6));
+err_blink:
+	gpio_free(S3C2410_GPA(4));
+err_blink_green:
+	gpio_free(S3C2410_GPA(3));
+err_blink_red:
+	return ret;
+}
+
+static int rx1950leds_remove(struct platform_device *pdev)
+{
+	led_classdev_unregister(&rx1950_greenled);
+	led_classdev_unregister(&rx1950_redled);
+	led_classdev_unregister(&rx1950_blueled);
+	gpio_free(S3C2410_GPA(11));
+	gpio_free(S3C2410_GPA(6));
+	gpio_free(S3C2410_GPA(7));
+	gpio_free(S3C2410_GPJ(6));
+	gpio_free(S3C2410_GPA(3));
+	gpio_free(S3C2410_GPA(4));
+	return 0;
+}
+
+
+static struct platform_driver rx1950leds_driver = {
+	.driver		= {
+		.name	= "rx1950-leds",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= rx1950leds_probe,
+	.remove		= rx1950leds_remove,
+};
+
+
+static int __init rx1950leds_init(void)
+{
+	return platform_driver_register(&rx1950leds_driver);
+}
+
+static void __exit rx1950leds_exit(void)
+{
+	platform_driver_unregister(&rx1950leds_driver);
+}
+
+module_init(rx1950leds_init);
+module_exit(rx1950leds_exit);
+
+MODULE_AUTHOR("Vasily Khoruzhick <anarsoul@gmail.com>");
+MODULE_DESCRIPTION("LED driver for the iPAQ RX1950");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:rx1950-leds");
-- 
1.7.1

[PATCH 2/3] Add s3c-adc-battery driver

[Vasily Khoruzhick]

s3c-adc-battery is driver for monitoring
and charging battery on iPAQ H1930/H1940/RX1950.
It depends on s3c-adc driver to get battery voltage and current.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 drivers/power/Kconfig           |    6 +
 drivers/power/Makefile          |    1 +
 drivers/power/s3c_adc_battery.c |  518 +++++++++++++++++++++++++++++++++++++++
 include/linux/s3c_adc_battery.h |   37 +++
 4 files changed, 562 insertions(+), 0 deletions(-)
 create mode 100644 drivers/power/s3c_adc_battery.c
 create mode 100644 include/linux/s3c_adc_battery.h

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 8e9ba17..645baa5 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -136,6 +136,12 @@ config BATTERY_Z2
 	help
 	  Say Y to include support for the battery on the Zipit Z2.
 
+config BATTERY_S3C_ADC
+	tristate "Battery driver for Samsung ADC based monitoring"
+	depends on S3C_ADC
+	help
+	  Say Y here to enable support for iPAQ h1930/h1940/rx1950 battery
+
 config CHARGER_PCF50633
 	tristate "NXP PCF50633 MBC"
 	depends on MFD_PCF50633
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 0005080..166bcbf 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -33,4 +33,5 @@ obj-$(CONFIG_BATTERY_BQ27x00)	+= bq27x00_battery.o
 obj-$(CONFIG_BATTERY_DA9030)	+= da9030_battery.o
 obj-$(CONFIG_BATTERY_MAX17040)	+= max17040_battery.o
 obj-$(CONFIG_BATTERY_Z2)	+= z2_battery.o
+obj-$(CONFIG_BATTERY_S3C_ADC)	+= s3c_adc_battery.o
 obj-$(CONFIG_CHARGER_PCF50633)	+= pcf50633-charger.o
diff --git a/drivers/power/s3c_adc_battery.c b/drivers/power/s3c_adc_battery.c
new file mode 100644
index 0000000..d2b729b
--- /dev/null
+++ b/drivers/power/s3c_adc_battery.c
@@ -0,0 +1,518 @@
+/*
+ * drivers/power/s3c_adc_battery.c
+ *	Copyright (c) Vasily Khoruzhick
+ *	Based on h1940_battery.c by Arnaud Patard
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file COPYING in the main directory of this archive for
+ * more details.
+ *
+ *	    iPAQ h1930/h1940/rx1950 battery controler driver
+ */
+
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/leds.h>
+#include <linux/gpio.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/timer.h>
+#include <linux/jiffies.h>
+#include <linux/s3c_adc_battery.h>
+
+#include <asm/irq.h>
+#include <mach/regs-gpio.h>
+#include <mach/regs-gpioj.h>
+
+#include <plat/adc.h>
+
+#define BAT_POLL_INTERVAL			10000 /* ms */
+#define JITTER_DELAY				500 /* ms */
+
+struct s3c_adc_bat {
+	struct power_supply	psy;
+	struct s3c_adc_client	*client;
+	struct s3c_adc_batt_pdata *pdata;
+};
+
+#ifdef CONFIG_LEDS_CLASS
+DEFINE_LED_TRIGGER(charger_led_trigger);
+#endif
+
+static int s3c_adc_ac_get_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val);
+
+static char *s3c_adc_supplied_to[] = {
+	"main-battery",
+};
+
+static enum power_supply_property s3c_adc_ac_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+};
+
+static struct power_supply s3c_adc_ac = {
+	.name = "ac",
+	.type = POWER_SUPPLY_TYPE_MAINS,
+	.supplied_to = s3c_adc_supplied_to,
+	.num_supplicants = ARRAY_SIZE(s3c_adc_supplied_to),
+	.properties = s3c_adc_ac_props,
+	.num_properties = ARRAY_SIZE(s3c_adc_ac_props),
+	.get_property = s3c_adc_ac_get_property,
+};
+
+static enum power_supply_property s3c_adc_backup_bat_props[] = {
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+};
+
+static int s3c_adc_backup_bat_get_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val);
+
+static struct s3c_adc_bat backup_bat = {
+	.psy = {
+		.name		= "backup-battery",
+		.type		= POWER_SUPPLY_TYPE_BATTERY,
+		.properties	= s3c_adc_backup_bat_props,
+		.num_properties = ARRAY_SIZE(s3c_adc_backup_bat_props),
+		.get_property	= s3c_adc_backup_bat_get_property,
+		.use_for_apm	= 1,
+	},
+};
+
+static enum power_supply_property s3c_adc_main_bat_props[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
+	POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN,
+	POWER_SUPPLY_PROP_CHARGE_NOW,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+};
+
+static int s3c_adc_bat_get_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val);
+
+static struct s3c_adc_bat main_bat = {
+	.psy = {
+		.name		= "main-battery",
+		.type		= POWER_SUPPLY_TYPE_BATTERY,
+		.properties	= s3c_adc_main_bat_props,
+		.num_properties = ARRAY_SIZE(s3c_adc_main_bat_props),
+		.get_property	= s3c_adc_bat_get_property,
+		.use_for_apm	= 1,
+	},
+};
+
+static int s3c_adc_ac_get_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val)
+{
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval =
+			!gpio_get_value(main_bat.pdata->gpio_cable_plugged);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}
+
+static int s3c_adc_backup_bat_get_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val)
+{
+	struct s3c_adc_bat *bat = container_of(psy, struct s3c_adc_bat, psy);
+	static int volt_value = -1;
+	static unsigned int timestamp;
+
+	if (!bat) {
+		printk(KERN_ERR "%s: no battery infos ?!\n", __func__);
+		return -EINVAL;
+	}
+
+	if (volt_value < 0 ||
+		jiffies_to_msecs(jiffies - timestamp) > BAT_POLL_INTERVAL) {
+		volt_value = s3c_adc_read(bat->client,
+			bat->pdata->backup_volt_channel);
+		volt_value *= bat->pdata->backup_volt_mult;
+		timestamp = jiffies;
+	}
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = volt_value;
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+		val->intval = bat->pdata->backup_volt_min;
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		val->intval = bat->pdata->backup_volt_max;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static void cable_plugged_handler(unsigned long data)
+{
+	static int old_status;
+	int is_plugged = !gpio_get_value(main_bat.pdata->gpio_cable_plugged);
+
+	if (old_status == is_plugged)
+		return;
+
+	printk(KERN_INFO "Power cable %s\n",\
+			is_plugged ? "plugged" : "unplugged");
+	if (is_plugged) {
+		main_bat.pdata->enable_charger();
+#ifdef CONFIG_LEDS_CLASS
+		led_trigger_event(charger_led_trigger, LED_HALF);
+#endif
+	} else {
+		main_bat.pdata->disable_charger();
+#ifdef CONFIG_LEDS_CLASS
+		led_trigger_event(charger_led_trigger, LED_OFF);
+#endif
+	}
+
+	old_status = is_plugged;
+	power_supply_changed(&s3c_adc_ac);
+}
+
+static void charge_finished_handler(unsigned long data)
+{
+	static int old_status;
+	int is_plugged = !gpio_get_value(main_bat.pdata->gpio_cable_plugged);
+	int is_charged = gpio_get_value(main_bat.pdata->gpio_charge_finished);
+
+	if (old_status == is_charged)
+		return;
+
+	if (!is_plugged)
+		return;
+
+	if (is_charged) {
+#ifdef CONFIG_LEDS_CLASS
+		led_trigger_event(charger_led_trigger, LED_FULL);
+#endif
+		main_bat.pdata->disable_charger();
+	} else {
+#ifdef CONFIG_LEDS_CLASS
+		led_trigger_event(charger_led_trigger, LED_HALF);
+#endif
+		main_bat.pdata->enable_charger();
+	}
+
+	old_status = is_charged;
+	power_supply_changed(&main_bat.psy);
+}
+
+static DEFINE_TIMER(cable_plugged_timer, cable_plugged_handler, 0, 0);
+static DEFINE_TIMER(charge_finished_timer, charge_finished_handler, 0, 0);
+
+static irqreturn_t s3c_adc_batt_cable(int irq, void *dev_id)
+{
+	mod_timer(&cable_plugged_timer,
+		jiffies + msecs_to_jiffies(JITTER_DELAY));
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t s3c_adc_batt_charged(int irq, void *dev_id)
+{
+	mod_timer(&charge_finished_timer,
+		jiffies + msecs_to_jiffies(JITTER_DELAY));
+	return IRQ_HANDLED;
+}
+
+static int calc_full_volt(int volt_val, int cur_val)
+{
+	return volt_val +
+		cur_val * main_bat.pdata->internal_impedance / 1000;
+}
+
+static int s3c_adc_bat_get_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val)
+{
+	struct s3c_adc_bat *bat = container_of(psy, struct s3c_adc_bat, psy);
+	static int volt_value = -1, cur_value = -1;
+	static unsigned int timestamp;
+	static int level = 1000000;
+
+	int status = POWER_SUPPLY_STATUS_DISCHARGING, new_level, full_volt;
+	const struct s3c_adc_batt_thresh *lut = bat->pdata->lut_noac;
+	unsigned int lut_size = bat->pdata->lut_noac_cnt;
+
+	if (!bat) {
+		printk(KERN_ERR "%s: no battery infos ?!\n", __func__);
+		return -EINVAL;
+	}
+
+	if (volt_value < 0 || cur_value < 0 ||
+		jiffies_to_msecs(jiffies - timestamp) > BAT_POLL_INTERVAL) {
+		volt_value = s3c_adc_read(bat->client,
+			bat->pdata->volt_channel) * bat->pdata->volt_mult;
+		cur_value = s3c_adc_read(bat->client,
+			bat->pdata->current_channel) * bat->pdata->current_mult;
+		timestamp = jiffies;
+	}
+
+	if (!gpio_get_value(bat->pdata->gpio_cable_plugged)) {
+		if (gpio_get_value(bat->pdata->gpio_charge_finished))
+			status = POWER_SUPPLY_STATUS_FULL;
+		else {
+			status = POWER_SUPPLY_STATUS_CHARGING;
+			lut = bat->pdata->lut_acin;
+			lut_size = bat->pdata->lut_acin_cnt;
+		}
+	}
+
+	new_level = 100000;
+	full_volt = calc_full_volt((volt_value / 1000), (cur_value / 1000));
+
+	if (full_volt < calc_full_volt(lut->volt, lut->cur)) {
+		lut_size--;
+		while (lut_size--) {
+			int lut_volt1, lut_volt2;
+			lut_volt1 = calc_full_volt(lut[0].volt, lut[0].cur);
+			lut_volt2 = calc_full_volt(lut[1].volt, lut[1].cur);
+			if (full_volt < lut_volt1 && full_volt >= lut_volt2) {
+				new_level = (lut[1].level +
+					(lut[0].level - lut[1].level) *
+					(full_volt - lut_volt2) /
+					(lut_volt1 - lut_volt2)) * 1000;
+				break;
+			}
+			new_level = lut[1].level * 1000;
+			lut++;
+		}
+	}
+
+#if 0
+	/* Battery level can't go up during discharging, and
+	 * can't go down during charging
+	 */
+	if ((status == POWER_SUPPLY_STATUS_DISCHARGING && new_level < level) ||
+		(status == POWER_SUPPLY_STATUS_CHARGING && new_level > level) ||
+		status == POWER_SUPPLY_STATUS_FULL)
+		level = new_level;
+#else
+	level = new_level;
+#endif
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = status;
+		return 0;
+	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+		val->intval = 100000;
+		return 0;
+	case POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN:
+		val->intval = 0;
+		return 0;
+	case POWER_SUPPLY_PROP_CHARGE_NOW:
+		val->intval = level;
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = volt_value;
+		return 0;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		val->intval = cur_value;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int __init s3c_adc_batt_probe(struct platform_device *pdev)
+{
+	struct s3c_adc_client	*client;
+	struct s3c_adc_batt_pdata *pdata = pdev->dev.platform_data;
+	int ret;
+
+	client = s3c_adc_register(pdev, NULL, NULL, 0);
+	if (IS_ERR(client)) {
+		dev_err(&pdev->dev, "cannot register adc\n");
+		return PTR_ERR(client);
+	}
+
+	platform_set_drvdata(pdev, client);
+
+	main_bat.client = client;
+	main_bat.pdata = pdev->dev.platform_data;
+
+	ret = power_supply_register(&pdev->dev, &main_bat.psy);
+	if (ret)
+		goto err_reg_main;
+	if (pdata->backup_volt_mult) {
+		backup_bat.client = client;
+		backup_bat.pdata = pdev->dev.platform_data;
+		ret = power_supply_register(&pdev->dev, &backup_bat.psy);
+		if (ret)
+			goto err_reg_backup;
+	}
+	ret = power_supply_register(&pdev->dev, &s3c_adc_ac);
+	if (ret)
+		goto err_reg_ac;
+
+
+	ret = gpio_request(main_bat.pdata->gpio_cable_plugged, "batcable");
+	if (ret)
+		goto err_gpio1;
+
+	ret = request_irq(gpio_to_irq(main_bat.pdata->gpio_cable_plugged),
+			s3c_adc_batt_cable,
+			IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+			pdev->name, NULL);
+	if (ret)
+		goto err_irq1;
+
+	ret = gpio_request(main_bat.pdata->gpio_charge_finished, "charged");
+	if (ret)
+		goto err_gpio2;
+
+	ret = request_irq(gpio_to_irq(main_bat.pdata->gpio_charge_finished),
+			s3c_adc_batt_charged,
+			IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+			pdev->name, NULL);
+	if (ret)
+		goto err_irq2;
+
+	ret = pdata->init();
+	if (ret)
+		goto err_platform;
+
+#ifdef CONFIG_LEDS_CLASS
+	led_trigger_register_simple("s3c-adc-charger", &charger_led_trigger);
+#endif
+	printk(KERN_INFO "s3c-adc-batt successfully loaded\n");
+
+	device_init_wakeup(&pdev->dev, 1);
+
+	/* Schedule timer to check current status */
+	mod_timer(&cable_plugged_timer,
+		jiffies + msecs_to_jiffies(JITTER_DELAY));
+
+	return 0;
+
+err_platform:
+	free_irq(gpio_to_irq(main_bat.pdata->gpio_charge_finished), NULL);
+	del_timer_sync(&charge_finished_timer);
+err_irq2:
+	gpio_free(main_bat.pdata->gpio_charge_finished);
+err_gpio2:
+	free_irq(gpio_to_irq(main_bat.pdata->gpio_cable_plugged), NULL);
+	del_timer_sync(&cable_plugged_timer);
+err_irq1:
+	gpio_free(main_bat.pdata->gpio_cable_plugged);
+err_gpio1:
+	power_supply_unregister(&s3c_adc_ac);
+err_reg_ac:
+	if (pdata->backup_volt_mult)
+		power_supply_unregister(&backup_bat.psy);
+err_reg_backup:
+	power_supply_unregister(&main_bat.psy);
+err_reg_main:
+	return ret;
+}
+
+static int s3c_adc_batt_remove(struct platform_device *pdev)
+{
+	struct s3c_adc_client *client = platform_get_drvdata(pdev);
+	struct s3c_adc_batt_pdata *pdata = pdev->dev.platform_data;
+
+#ifdef CONFIG_LEDS_CLASS
+	led_trigger_unregister_simple(charger_led_trigger);
+#endif
+
+	power_supply_unregister(&s3c_adc_ac);
+	power_supply_unregister(&main_bat.psy);
+	if (pdata->backup_volt_mult)
+		power_supply_unregister(&backup_bat.psy);
+
+	s3c_adc_release(client);
+
+	free_irq(gpio_to_irq(pdata->gpio_charge_finished), NULL);
+	gpio_free(pdata->gpio_charge_finished);
+	del_timer_sync(&charge_finished_timer);
+	free_irq(gpio_to_irq(pdata->gpio_cable_plugged), NULL);
+	gpio_free(pdata->gpio_cable_plugged);
+	del_timer_sync(&cable_plugged_timer);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int s3c_adc_batt_suspend(struct platform_device *pdev,
+	pm_message_t state)
+{
+	struct s3c_adc_batt_pdata *pdata = pdev->dev.platform_data;
+	if (device_may_wakeup(&pdev->dev)) {
+		enable_irq_wake(gpio_to_irq(pdata->gpio_charge_finished));
+		enable_irq_wake(gpio_to_irq(pdata->gpio_cable_plugged));
+	} else {
+		disable_irq(gpio_to_irq(pdata->gpio_charge_finished));
+		disable_irq(gpio_to_irq(pdata->gpio_cable_plugged));
+		main_bat.pdata->disable_charger();
+	}
+
+	return 0;
+}
+
+static int s3c_adc_batt_resume(struct platform_device *pdev)
+{
+	struct s3c_adc_batt_pdata *pdata = pdev->dev.platform_data;
+	if (device_may_wakeup(&pdev->dev)) {
+		disable_irq_wake(gpio_to_irq(pdata->gpio_charge_finished));
+		disable_irq_wake(gpio_to_irq(pdata->gpio_cable_plugged));
+	} else {
+		enable_irq(gpio_to_irq(pdata->gpio_charge_finished));
+		enable_irq(gpio_to_irq(pdata->gpio_cable_plugged));
+	}
+
+	/* Schedule timer to check current status */
+	mod_timer(&cable_plugged_timer,
+		jiffies + msecs_to_jiffies(JITTER_DELAY));
+
+	return 0;
+}
+#else
+#define s3c_adc_battery_suspend NULL
+#define s3c_adc_battery_resume NULL
+#endif
+
+static struct platform_driver s3c_adc_batt_driver = {
+	.driver		= {
+		.name	= "s3c-adc-battery",
+	},
+	.probe		= s3c_adc_batt_probe,
+	.remove		= s3c_adc_batt_remove,
+	.suspend	= s3c_adc_batt_suspend,
+	.resume		= s3c_adc_batt_resume,
+};
+
+
+static int __init s3c_adc_batt_init(void)
+{
+	return platform_driver_register(&s3c_adc_batt_driver);
+}
+
+static void __exit s3c_adc_batt_exit(void)
+{
+	platform_driver_unregister(&s3c_adc_batt_driver);
+}
+
+module_init(s3c_adc_batt_init);
+module_exit(s3c_adc_batt_exit);
+
+MODULE_AUTHOR("Vasily Khoruzhick <anarsoul@gmail.com>");
+MODULE_DESCRIPTION("iPAQ H1930/H1940/RX1950 battery controler driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/s3c_adc_battery.h b/include/linux/s3c_adc_battery.h
new file mode 100644
index 0000000..a8dad74
--- /dev/null
+++ b/include/linux/s3c_adc_battery.h
@@ -0,0 +1,37 @@
+#ifndef _S3C_ADC_BATTERY_H
+#define _S3C_ADC_BATTERY_H
+
+struct s3c_adc_batt_thresh {
+	int volt; /* mV */
+	int cur; /* mA */
+	int level; /* percent */
+};
+
+struct s3c_adc_batt_pdata {
+	int (*init)(void);
+	void (*exit)(void);
+	void (*enable_charger)(void);
+	void (*disable_charger)(void);
+
+	unsigned int gpio_cable_plugged;
+	unsigned int gpio_charge_finished;
+
+	const struct s3c_adc_batt_thresh *lut_noac;
+	unsigned int lut_noac_cnt;
+	const struct s3c_adc_batt_thresh *lut_acin;
+	unsigned int lut_acin_cnt;
+
+	const unsigned int volt_channel;
+	const unsigned int current_channel;
+	const unsigned int backup_volt_channel;
+
+	const unsigned int volt_mult;
+	const unsigned int current_mult;
+	const unsigned int backup_volt_mult;
+	const unsigned int internal_impedance;
+
+	const unsigned int backup_volt_max;
+	const unsigned int backup_volt_min;
+};
+
+#endif
-- 
1.7.1

[PATCH 3/3] rx1950: add battery device

[Vasily Khoruzhick]

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-s3c2440/Kconfig       |    2 +
 arch/arm/mach-s3c2440/mach-rx1950.c |  111 +++++++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-s3c2440/Kconfig b/arch/arm/mach-s3c2440/Kconfig
index cd8e7de..ee3137e 100644
--- a/arch/arm/mach-s3c2440/Kconfig
+++ b/arch/arm/mach-s3c2440/Kconfig
@@ -198,6 +198,8 @@ config MACH_RX1950
 	select S3C_DEV_NAND
 	select S3C2410_IOTIMING if S3C2440_CPUFREQ
 	select S3C2440_XTAL_16934400
+	select S3C_ADC
+	select BATTERY_S3C_ADC
 	help
 	   Say Y here if you're using HP iPAQ rx1950
 
diff --git a/arch/arm/mach-s3c2440/mach-rx1950.c b/arch/arm/mach-s3c2440/mach-rx1950.c
index 55b806a..a335eb9 100644
--- a/arch/arm/mach-s3c2440/mach-rx1950.c
+++ b/arch/arm/mach-s3c2440/mach-rx1950.c
@@ -26,6 +26,7 @@
 #include <linux/sysdev.h>
 #include <linux/pwm_backlight.h>
 #include <linux/pwm.h>
+#include <linux/s3c_adc_battery.h>
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
@@ -131,6 +132,115 @@ static struct platform_device rx1950_device_leds = {
 	.id               = -1,
 };
 
+static const struct s3c_adc_batt_thresh batt_lut_noac[] = {
+	{4100, 156, 100},
+	{4050, 156, 95},
+	{4025, 141, 90},
+	{3995, 144, 85},
+	{3957, 162, 80},
+	{3931, 147, 75},
+	{3902, 147, 70},
+	{3863, 153, 65},
+	{3838, 150, 60},
+	{3800, 153, 55},
+	{3765, 153, 50},
+	{3748, 172, 45},
+	{3740, 153, 40},
+	{3714, 175, 35},
+	{3710, 156, 30},
+	{3963, 156, 25},
+	{3672, 178, 20},
+	{3651, 178, 15},
+	{3629, 178, 10},
+	{3612, 162, 5},
+	{3605, 162, 0},
+};
+
+static const struct s3c_adc_batt_thresh batt_lut_acin[] = {
+	{4200, 0, 100},
+	{4190, 0, 99},
+	{4178, 0, 95},
+	{4110, 0, 70},
+	{4076, 0, 65},
+	{4046, 0, 60},
+	{4021, 0, 55},
+	{3999, 0, 50},
+	{3982, 0, 45},
+	{3965, 0, 40},
+	{3957, 0, 35},
+	{3948, 0, 30},
+	{3936, 0, 25},
+	{3927, 0, 20},
+	{3906, 0, 15},
+	{3880, 0, 10},
+	{3829, 0, 5},
+	{3820, 0, 0},
+};
+
+int rx1950_batt_init(void)
+{
+	int ret;
+
+	ret = gpio_request(S3C2410_GPJ(2), "rx1950-charger-enable-1");
+	if (ret)
+		goto err_gpio1;
+	ret = gpio_request(S3C2410_GPJ(3), "rx1950-charger-enable-2");
+	if (ret)
+		goto err_gpio2;
+
+	return 0;
+
+err_gpio2:
+	gpio_free(S3C2410_GPJ(2));
+err_gpio1:
+	return ret;
+}
+
+void rx1950_batt_exit(void)
+{
+	gpio_free(S3C2410_GPJ(2));
+	gpio_free(S3C2410_GPJ(3));
+}
+
+void rx1950_enable_charger(void)
+{
+	gpio_direction_output(S3C2410_GPJ(2), 1);
+	gpio_direction_output(S3C2410_GPJ(3), 1);
+}
+
+void rx1950_disable_charger(void)
+{
+	gpio_direction_output(S3C2410_GPJ(2), 0);
+	gpio_direction_output(S3C2410_GPJ(3), 0);
+}
+
+static struct s3c_adc_batt_pdata rx1950_batt_cfg = {
+	.init = rx1950_batt_init,
+	.exit = rx1950_batt_exit,
+	.enable_charger = rx1950_enable_charger,
+	.disable_charger = rx1950_disable_charger,
+	.gpio_cable_plugged = S3C2410_GPF(2),
+	.gpio_charge_finished = S3C2410_GPF(3),
+	.lut_noac = batt_lut_noac,
+	.lut_noac_cnt = ARRAY_SIZE(batt_lut_noac),
+	.lut_acin = batt_lut_acin,
+	.lut_acin_cnt = ARRAY_SIZE(batt_lut_acin),
+	.volt_channel = 0,
+	.current_channel = 1,
+	.volt_mult = 4235,
+	.current_mult = 2900,
+	.internal_impedance = 200,
+};
+
+static struct platform_device rx1950_device_battery = {
+	.name             = "s3c-adc-battery",
+	.id               = -1,
+	.dev = {
+		.parent = &s3c_device_adc.dev,
+		.platform_data = &rx1950_batt_cfg,
+	},
+};
+
 static struct s3c2410fb_mach_info rx1950_lcd_cfg = {
 	.displays = &rx1950_display,
 	.num_displays = 1,
@@ -508,6 +618,7 @@ static struct platform_device *rx1950_devices[] __initdata = {
 	&rx1950_backlight,
 	&rx1950_device_gpiokeys,
 	&rx1950_device_leds,
+	&rx1950_device_battery,
 };
 
 static struct clk *rx1950_clocks[] __initdata = {
-- 
1.7.1

[PATCH 2/3] Add s3c-adc-battery driver

[Anton Vorontsov]

On Fri, Jul 09, 2010 at 02:27:21PM +0300, Vasily Khoruzhick wrote:
> s3c-adc-battery is driver for monitoring
> and charging battery on iPAQ H1930/H1940/RX1950.
> It depends on s3c-adc driver to get battery voltage and current.
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

Thanks for the driver!

Please Cc me on driver/power/* patches, it's pure luck that I
noticed this patch.

> diff --git a/drivers/power/s3c_adc_battery.c b/drivers/power/s3c_adc_battery.c
> new file mode 100644
> index 0000000..d2b729b
> --- /dev/null
> +++ b/drivers/power/s3c_adc_battery.c
> @@ -0,0 +1,518 @@
> +/*
> + * drivers/power/s3c_adc_battery.c

This line isn't needed. You'd better move the
"iPAQ h1930/h1940/rx1950 battery controler driver" line here.

> + *	Copyright (c) Vasily Khoruzhick
> + *	Based on h1940_battery.c by Arnaud Patard
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file COPYING in the main directory of this archive for
> + * more details.
> + *
> + *	    iPAQ h1930/h1940/rx1950 battery controler driver
> + */

#include <linux/init.h> for __init
#include <linux/errno.h> for E-codes

> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/leds.h>
> +#include <linux/gpio.h>
> +#include <linux/err.h>
> +#include <linux/io.h>

Do you really need io.h?

> +#include <linux/timer.h>
> +#include <linux/jiffies.h>
> +#include <linux/s3c_adc_battery.h>
> +
> +#include <asm/irq.h>

Is this really needed?

> +#include <mach/regs-gpio.h>
> +#include <mach/regs-gpioj.h>

Not linux/gpio.h?

> +
> +#include <plat/adc.h>
> +
> +#define BAT_POLL_INTERVAL			10000 /* ms */
> +#define JITTER_DELAY				500 /* ms */
> +
> +struct s3c_adc_bat {
> +	struct power_supply	psy;
> +	struct s3c_adc_client	*client;
> +	struct s3c_adc_batt_pdata *pdata;
> +};
> +
> +#ifdef CONFIG_LEDS_CLASS
> +DEFINE_LED_TRIGGER(charger_led_trigger);
> +#endif
> +
> +static int s3c_adc_ac_get_property(struct power_supply *psy,
> +				enum power_supply_property psp,
> +				union power_supply_propval *val);

Please avoid forward declarations where possible.

> +static char *s3c_adc_supplied_to[] = {
> +	"main-battery",
> +};
> +
> +static enum power_supply_property s3c_adc_ac_props[] = {
> +	POWER_SUPPLY_PROP_ONLINE,
> +};
> +
> +static struct power_supply s3c_adc_ac = {
> +	.name = "ac",
> +	.type = POWER_SUPPLY_TYPE_MAINS,
> +	.supplied_to = s3c_adc_supplied_to,
> +	.num_supplicants = ARRAY_SIZE(s3c_adc_supplied_to),
> +	.properties = s3c_adc_ac_props,
> +	.num_properties = ARRAY_SIZE(s3c_adc_ac_props),
> +	.get_property = s3c_adc_ac_get_property,
> +};

You really shouldn't create yet another "ac" power supply
instance. You can use drivers/power/pda_power.c for this.

> +
> +static enum power_supply_property s3c_adc_backup_bat_props[] = {
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_VOLTAGE_MIN,
> +	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
> +};
> +
> +static int s3c_adc_backup_bat_get_property(struct power_supply *psy,
> +				enum power_supply_property psp,
> +				union power_supply_propval *val);
> +
> +static struct s3c_adc_bat backup_bat = {
> +	.psy = {
> +		.name		= "backup-battery",
> +		.type		= POWER_SUPPLY_TYPE_BATTERY,
> +		.properties	= s3c_adc_backup_bat_props,
> +		.num_properties = ARRAY_SIZE(s3c_adc_backup_bat_props),
> +		.get_property	= s3c_adc_backup_bat_get_property,
> +		.use_for_apm	= 1,
> +	},
> +};
> +
> +static enum power_supply_property s3c_adc_main_bat_props[] = {
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
> +	POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN,
> +	POWER_SUPPLY_PROP_CHARGE_NOW,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +};
> +
> +static int s3c_adc_bat_get_property(struct power_supply *psy,
> +				enum power_supply_property psp,
> +				union power_supply_propval *val);
> +
> +static struct s3c_adc_bat main_bat = {
> +	.psy = {
> +		.name		= "main-battery",
> +		.type		= POWER_SUPPLY_TYPE_BATTERY,
> +		.properties	= s3c_adc_main_bat_props,
> +		.num_properties = ARRAY_SIZE(s3c_adc_main_bat_props),
> +		.get_property	= s3c_adc_bat_get_property,
> +		.use_for_apm	= 1,
> +	},
> +};
> +
> +static int s3c_adc_ac_get_property(struct power_supply *psy,
> +				enum power_supply_property psp,
> +				union power_supply_propval *val)
> +{
> +	int ret = 0;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		val->intval =
> +			!gpio_get_value(main_bat.pdata->gpio_cable_plugged);

Yep, it's duplication of pda_power.c functionality.

So, you probably want to register S3C ADC battery and pda-power
platform devices. And S3C ADC battery driver would only do
battery stuff, and pda-power would do the AC stuff.

> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +	return ret;
> +}
> +
> +static int s3c_adc_backup_bat_get_property(struct power_supply *psy,
> +				enum power_supply_property psp,
> +				union power_supply_propval *val)
> +{
> +	struct s3c_adc_bat *bat = container_of(psy, struct s3c_adc_bat, psy);
> +	static int volt_value = -1;
> +	static unsigned int timestamp;
> +
> +	if (!bat) {
> +		printk(KERN_ERR "%s: no battery infos ?!\n", __func__);

dev_err()

> +		return -EINVAL;
> +	}
> +
> +	if (volt_value < 0 ||
> +		jiffies_to_msecs(jiffies - timestamp) > BAT_POLL_INTERVAL) {
> +		volt_value = s3c_adc_read(bat->client,
> +			bat->pdata->backup_volt_channel);
> +		volt_value *= bat->pdata->backup_volt_mult;
> +		timestamp = jiffies;
> +	}
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		val->intval = volt_value;
> +		return 0;
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> +		val->intval = bat->pdata->backup_volt_min;
> +		return 0;
> +	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> +		val->intval = bat->pdata->backup_volt_max;
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static void cable_plugged_handler(unsigned long data)
> +{
> +	static int old_status;
> +	int is_plugged = !gpio_get_value(main_bat.pdata->gpio_cable_plugged);
> +
> +	if (old_status == is_plugged)
> +		return;
> +
> +	printk(KERN_INFO "Power cable %s\n",\
> +			is_plugged ? "plugged" : "unplugged");

dev_info()

> +	if (is_plugged) {
> +		main_bat.pdata->enable_charger();
> +#ifdef CONFIG_LEDS_CLASS
> +		led_trigger_event(charger_led_trigger, LED_HALF);
> +#endif
> +	} else {
> +		main_bat.pdata->disable_charger();
> +#ifdef CONFIG_LEDS_CLASS
> +		led_trigger_event(charger_led_trigger, LED_OFF);
> +#endif

?

We have a generic charge triggers, see
drivers/power/power_supply_leds.c, so you probably don't need this.

> +	}
> +
> +	old_status = is_plugged;
> +	power_supply_changed(&s3c_adc_ac);
> +}
> +
> +static void charge_finished_handler(unsigned long data)
> +{
> +	static int old_status;
> +	int is_plugged = !gpio_get_value(main_bat.pdata->gpio_cable_plugged);
> +	int is_charged = gpio_get_value(main_bat.pdata->gpio_charge_finished);
> +
> +	if (old_status == is_charged)
> +		return;
> +
> +	if (!is_plugged)
> +		return;
> +
> +	if (is_charged) {
> +#ifdef CONFIG_LEDS_CLASS
> +		led_trigger_event(charger_led_trigger, LED_FULL);
> +#endif
> +		main_bat.pdata->disable_charger();
> +	} else {
> +#ifdef CONFIG_LEDS_CLASS
> +		led_trigger_event(charger_led_trigger, LED_HALF);
> +#endif
> +		main_bat.pdata->enable_charger();
> +	}
> +
> +	old_status = is_charged;
> +	power_supply_changed(&main_bat.psy);
> +}
> +
> +static DEFINE_TIMER(cable_plugged_timer, cable_plugged_handler, 0, 0);
> +static DEFINE_TIMER(charge_finished_timer, charge_finished_handler, 0, 0);
> +
> +static irqreturn_t s3c_adc_batt_cable(int irq, void *dev_id)
> +{
> +	mod_timer(&cable_plugged_timer,
> +		jiffies + msecs_to_jiffies(JITTER_DELAY));
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t s3c_adc_batt_charged(int irq, void *dev_id)
> +{
> +	mod_timer(&charge_finished_timer,
> +		jiffies + msecs_to_jiffies(JITTER_DELAY));
> +	return IRQ_HANDLED;
> +}
> +
> +static int calc_full_volt(int volt_val, int cur_val)
> +{
> +	return volt_val +
> +		cur_val * main_bat.pdata->internal_impedance / 1000;
> +}
> +
> +static int s3c_adc_bat_get_property(struct power_supply *psy,
> +				enum power_supply_property psp,
> +				union power_supply_propval *val)
> +{
> +	struct s3c_adc_bat *bat = container_of(psy, struct s3c_adc_bat, psy);
> +	static int volt_value = -1, cur_value = -1;

I don't think that using static variables here is safe. Why
you need them? There must be a better way.

> +	static unsigned int timestamp;
> +	static int level = 1000000;
> +

No need for this empty line.

> +	int status = POWER_SUPPLY_STATUS_DISCHARGING, new_level, full_volt;

Each declaration on it own line please, e.g.

int status = ...;
int new_level;
int full_volt;

> +	const struct s3c_adc_batt_thresh *lut = bat->pdata->lut_noac;
> +	unsigned int lut_size = bat->pdata->lut_noac_cnt;
> +
> +	if (!bat) {
> +		printk(KERN_ERR "%s: no battery infos ?!\n", __func__);

dev_err() please.

> +		return -EINVAL;
> +	}
> +
> +	if (volt_value < 0 || cur_value < 0 ||
> +		jiffies_to_msecs(jiffies - timestamp) > BAT_POLL_INTERVAL) {
> +		volt_value = s3c_adc_read(bat->client,
> +			bat->pdata->volt_channel) * bat->pdata->volt_mult;
> +		cur_value = s3c_adc_read(bat->client,
> +			bat->pdata->current_channel) * bat->pdata->current_mult;
> +		timestamp = jiffies;
> +	}
> +
> +	if (!gpio_get_value(bat->pdata->gpio_cable_plugged)) {
> +		if (gpio_get_value(bat->pdata->gpio_charge_finished))
> +			status = POWER_SUPPLY_STATUS_FULL;
> +		else {
> +			status = POWER_SUPPLY_STATUS_CHARGING;
> +			lut = bat->pdata->lut_acin;
> +			lut_size = bat->pdata->lut_acin_cnt;
> +		}
> +	}
> +
> +	new_level = 100000;
> +	full_volt = calc_full_volt((volt_value / 1000), (cur_value / 1000));
> +
> +	if (full_volt < calc_full_volt(lut->volt, lut->cur)) {
> +		lut_size--;
> +		while (lut_size--) {
> +			int lut_volt1, lut_volt2;

int lut_volt1;
int lut_volt2;
<empty line>
...

> +			lut_volt1 = calc_full_volt(lut[0].volt, lut[0].cur);
> +			lut_volt2 = calc_full_volt(lut[1].volt, lut[1].cur);
> +			if (full_volt < lut_volt1 && full_volt >= lut_volt2) {
> +				new_level = (lut[1].level +
> +					(lut[0].level - lut[1].level) *
> +					(full_volt - lut_volt2) /
> +					(lut_volt1 - lut_volt2)) * 1000;
> +				break;
> +			}
> +			new_level = lut[1].level * 1000;
> +			lut++;
> +		}
> +	}

Cool!

> +#if 0

Do you really need this dead code?

> +	/* Battery level can't go up during discharging, and
> +	 * can't go down during charging
> +	 */
> +	if ((status == POWER_SUPPLY_STATUS_DISCHARGING && new_level < level) ||
> +		(status == POWER_SUPPLY_STATUS_CHARGING && new_level > level) ||
> +		status == POWER_SUPPLY_STATUS_FULL)
> +		level = new_level;
> +#else
> +	level = new_level;
> +#endif
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_STATUS:
> +		val->intval = status;
> +		return 0;
> +	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
> +		val->intval = 100000;
> +		return 0;
> +	case POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN:
> +		val->intval = 0;
> +		return 0;
> +	case POWER_SUPPLY_PROP_CHARGE_NOW:
> +		val->intval = level;
> +		return 0;
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		val->intval = volt_value;
> +		return 0;
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		val->intval = cur_value;
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int __init s3c_adc_batt_probe(struct platform_device *pdev)
> +{
> +	struct s3c_adc_client	*client;
> +	struct s3c_adc_batt_pdata *pdata = pdev->dev.platform_data;
> +	int ret;
> +
> +	client = s3c_adc_register(pdev, NULL, NULL, 0);
> +	if (IS_ERR(client)) {
> +		dev_err(&pdev->dev, "cannot register adc\n");
> +		return PTR_ERR(client);
> +	}
> +
> +	platform_set_drvdata(pdev, client);
> +
> +	main_bat.client = client;
> +	main_bat.pdata = pdev->dev.platform_data;
> +
> +	ret = power_supply_register(&pdev->dev, &main_bat.psy);
> +	if (ret)
> +		goto err_reg_main;
> +	if (pdata->backup_volt_mult) {
> +		backup_bat.client = client;
> +		backup_bat.pdata = pdev->dev.platform_data;
> +		ret = power_supply_register(&pdev->dev, &backup_bat.psy);
> +		if (ret)
> +			goto err_reg_backup;
> +	}
> +	ret = power_supply_register(&pdev->dev, &s3c_adc_ac);
> +	if (ret)
> +		goto err_reg_ac;
> +
> +

One empty line is enough.

> +	ret = gpio_request(main_bat.pdata->gpio_cable_plugged, "batcable");
> +	if (ret)
> +		goto err_gpio1;
> +
> +	ret = request_irq(gpio_to_irq(main_bat.pdata->gpio_cable_plugged),
> +			s3c_adc_batt_cable,
> +			IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +			pdev->name, NULL);
> +	if (ret)
> +		goto err_irq1;
> +
> +	ret = gpio_request(main_bat.pdata->gpio_charge_finished, "charged");
> +	if (ret)
> +		goto err_gpio2;
> +
> +	ret = request_irq(gpio_to_irq(main_bat.pdata->gpio_charge_finished),
> +			s3c_adc_batt_charged,
> +			IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +			pdev->name, NULL);
> +	if (ret)
> +		goto err_irq2;
> +
> +	ret = pdata->init();
> +	if (ret)
> +		goto err_platform;
> +
> +#ifdef CONFIG_LEDS_CLASS
> +	led_trigger_register_simple("s3c-adc-charger", &charger_led_trigger);
> +#endif
> +	printk(KERN_INFO "s3c-adc-batt successfully loaded\n");

dev_info().

> +
> +	device_init_wakeup(&pdev->dev, 1);
> +
> +	/* Schedule timer to check current status */
> +	mod_timer(&cable_plugged_timer,
> +		jiffies + msecs_to_jiffies(JITTER_DELAY));

You probably don't need a timer for this.
Use schedule_work() instead.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2

[PATCH 2/3] Add s3c-adc-battery driver

[Vasily Khoruzhick]

? ????????? ?? 9 ???? 2010 15:57:01 ????? Anton Vorontsov ???????:
> Please Cc me on driver/power/* patches, it's pure luck that I
> noticed this patch.

Ok, I'll do :)

> > + * drivers/power/s3c_adc_battery.c
> 
> This line isn't needed. You'd better move the
> "iPAQ h1930/h1940/rx1950 battery controler driver" line here.

Ok

> #include <linux/init.h> for __init
> #include <linux/errno.h> for E-codes

init is not necessary, but errno is, I'm using some error codes (-EINVAL) in 
driver

> > +#include <linux/interrupt.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/power_supply.h>
> > +#include <linux/leds.h>
> > +#include <linux/gpio.h>
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> 
> Do you really need io.h?

Will check.

> > +#include <linux/timer.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/s3c_adc_battery.h>
> > +
> > +#include <asm/irq.h>
> 
> Is this really needed?

asm/irq.h is not necessary

> > +#include <mach/regs-gpio.h>
> > +#include <mach/regs-gpioj.h>
> 
> Not linux/gpio.h?

Will remove it

> > +static int s3c_adc_ac_get_property(struct power_supply *psy,
> > +				enum power_supply_property psp,
> > +				union power_supply_propval *val);
> 
> Please avoid forward declarations where possible.

Ok
 
> > +static char *s3c_adc_supplied_to[] = {
> > +	"main-battery",
> > +};
> > +
> > +static enum power_supply_property s3c_adc_ac_props[] = {
> > +	POWER_SUPPLY_PROP_ONLINE,
> > +};
> > +
> > +static struct power_supply s3c_adc_ac = {
> > +	.name = "ac",
> > +	.type = POWER_SUPPLY_TYPE_MAINS,
> > +	.supplied_to = s3c_adc_supplied_to,
> > +	.num_supplicants = ARRAY_SIZE(s3c_adc_supplied_to),
> > +	.properties = s3c_adc_ac_props,
> > +	.num_properties = ARRAY_SIZE(s3c_adc_ac_props),
> > +	.get_property = s3c_adc_ac_get_property,
> > +};
> 
> You really shouldn't create yet another "ac" power supply
> instance. You can use drivers/power/pda_power.c for this.
> 
> Yep, it's duplication of pda_power.c functionality.
> 
> So, you probably want to register S3C ADC battery and pda-power
> platform devices. And S3C ADC battery driver would only do
> battery stuff, and pda-power would do the AC stuff.

Nope, I can't use pda_power here, as it can't control whether battery is 
already charged (it's separate gpio pin, and I need to disable charger in this 
case)

> > +	if (!bat) {
> > +		printk(KERN_ERR "%s: no battery infos ?!\n", __func__);
> 
> dev_err()

Ok

> > +	printk(KERN_INFO "Power cable %s\n",\
> > +			is_plugged ? "plugged" : "unplugged");
> 
> dev_info()

Ok

> ?
> 
> We have a generic charge triggers, see
> drivers/power/power_supply_leds.c, so you probably don't need this.

Ok

> > +static int s3c_adc_bat_get_property(struct power_supply *psy,
> > +				enum power_supply_property psp,
> > +				union power_supply_propval *val)
> > +{
> > +	struct s3c_adc_bat *bat = container_of(psy, struct s3c_adc_bat, psy);
> > +	static int volt_value = -1, cur_value = -1;
> 
> I don't think that using static variables here is safe. Why
> you need them? There must be a better way.

Need them to cache voltage and current values (to prevent spamming adc), 
however I should move them to s3c_adc_bat struct.

> > +	static unsigned int timestamp;
> > +	static int level = 1000000;
> > +
> 
> No need for this empty line.
>
Ok

> > +	int status = POWER_SUPPLY_STATUS_DISCHARGING, new_level, full_volt;
> 
> Each declaration on it own line please, e.g.
> 
> int status = ...;
> int new_level;
> int full_volt;

Ok

> > +			lut_volt1 = calc_full_volt(lut[0].volt, lut[0].cur);
> > +			lut_volt2 = calc_full_volt(lut[1].volt, lut[1].cur);
> > +			if (full_volt < lut_volt1 && full_volt >= lut_volt2) {
> > +				new_level = (lut[1].level +
> > +					(lut[0].level - lut[1].level) *
> > +					(full_volt - lut_volt2) /
> > +					(lut_volt1 - lut_volt2)) * 1000;
> > +				break;
> > +			}
> > +			new_level = lut[1].level * 1000;
> > +			lut++;
> > +		}
> > +	}
> 
> Cool!

Is it some kind of sarcasm? :)

> > +#if 0
> 
> Do you really need this dead code?

Nope, I'll remove it.

> 
> You probably don't need a timer for this.
> Use schedule_work() instead.

Timer's used to prevet jitter on gpio pins, and here it's used to check pins 
state on startup. I don't see any reason to replace it with delayed work.

Thanks for review!

Regards
Vasily
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100709/34e2210c/attachment.sig>

[PATCH 2/3] Add s3c-adc-battery driver

[Anton Vorontsov]

On Fri, Jul 09, 2010 at 04:19:14PM +0300, Vasily Khoruzhick wrote:
[...]
> > > +	.get_property = s3c_adc_ac_get_property,
> > > +};
> > 
> > You really shouldn't create yet another "ac" power supply
> > instance. You can use drivers/power/pda_power.c for this.
> > 
> > Yep, it's duplication of pda_power.c functionality.
> > 
> > So, you probably want to register S3C ADC battery and pda-power
> > platform devices. And S3C ADC battery driver would only do
> > battery stuff, and pda-power would do the AC stuff.
> 
> Nope, I can't use pda_power here, as it can't control whether battery is 
> already charged (it's separate gpio pin, and I need to disable charger in this 
> case)

I guess you can remove all the cable_plugged handling from s3c
battery driver, and then use power_supply.external_power_changed
callback to get cable_plugged notification (you should fill
pda_power.supplied_to properly to make it work).

See drivers/power/ds2760_battery.c and arch/arm/mach-pxa/hx4700.c
as an example.

You probably will need to change .external_power_changed() hook
to accept 'pst' argument (the supply that caused the change),
so that you could write something like this:

s3c_external_power_changed(struct power_supply *psy,
			   struct power_supply *ext)
{
	...
	ext->get_property(ext, ONLINE, &val);
	s3c->is_plugged = val.intval;

	s3c_kick_cable_plugged_handler(s3c);
}

Or, instead of ext->get_property() you could just use
power_supply_is_system_supplied(). Not very elegant, but
should work.

[...]
> > > +			lut_volt1 = calc_full_volt(lut[0].volt, lut[0].cur);
> > > +			lut_volt2 = calc_full_volt(lut[1].volt, lut[1].cur);
> > > +			if (full_volt < lut_volt1 && full_volt >= lut_volt2) {
> > > +				new_level = (lut[1].level +
> > > +					(lut[0].level - lut[1].level) *
> > > +					(full_volt - lut_volt2) /
> > > +					(lut_volt1 - lut_volt2)) * 1000;
> > > +				break;
> > > +			}
> > > +			new_level = lut[1].level * 1000;
> > > +			lut++;
> > > +		}
> > > +	}
> > 
> > Cool!
> 
> Is it some kind of sarcasm? :)

Nope. ;-)

[...]
> > You probably don't need a timer for this.
> > Use schedule_work() instead.
> 
> Timer's used to prevet jitter on gpio pins, and here it's used to check pins 
> state on startup. I don't see any reason to replace it with delayed work.

Timer is a softirq, atomic context. It adds latency to your
embedded, underpowered device. Plus with timer you can't use
sleepable GPIOs (we should change it for pda_power too, btw).

Jitter filtering isn't urgent task for the kernel (or user)
needs, right? So it's fine to postpone it for non-atomic
context.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2

[PATCH 2/3] Add s3c-adc-battery driver

[Vasily Khoruzhick]

? ????????? ?? 9 ???? 2010 16:53:45 ????? Anton Vorontsov ???????:
> 
> I guess you can remove all the cable_plugged handling from s3c
> battery driver, and then use power_supply.external_power_changed
> callback to get cable_plugged notification (you should fill
> pda_power.supplied_to properly to make it work).
> 
> See drivers/power/ds2760_battery.c and arch/arm/mach-pxa/hx4700.c
> as an example.
> 
> You probably will need to change .external_power_changed() hook
> to accept 'pst' argument (the supply that caused the change),
> so that you could write something like this:
> 
> s3c_external_power_changed(struct power_supply *psy,
> 			   struct power_supply *ext)
> {
> 	...
> 	ext->get_property(ext, ONLINE, &val);
> 	s3c->is_plugged = val.intval;
> 
> 	s3c_kick_cable_plugged_handler(s3c);
> }
> 
> Or, instead of ext->get_property() you could just use
> power_supply_is_system_supplied(). Not very elegant, but
> should work.

You didn't get the point. Here's workflow:

cable plugged -> GPF2 goes to 0, irq is generated
cable plugged irq handler -> cable_plugged ? enable_charger : disable_charger
charging....
battery is charged, GPF3 goes to 0, irq is generated
battery charged irq handler -> disable_charger
cable unplugged -> GPF2 goest to 1, irq is generated

pda_power driver does not support 'battery_charged' pin handling, but I need 
to handle this pin (and IRQ from it) to prevent battery overcharge

> Timer is a softirq, atomic context. It adds latency to your
> embedded, underpowered device. Plus with timer you can't use
> sleepable GPIOs (we should change it for pda_power too, btw).
> 
> Jitter filtering isn't urgent task for the kernel (or user)
> needs, right? So it's fine to postpone it for non-atomic
> context.

Ok, will replace timer with delayed works

Regards
Vasily
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100709/0dfa45f9/attachment.sig>

[PATCH 2/3] Add s3c-adc-battery driver

[Anton Vorontsov]

On Fri, Jul 09, 2010 at 05:07:43PM +0300, Vasily Khoruzhick wrote:
> ? ????????? ?? 9 ???? 2010 16:53:45 ????? Anton Vorontsov ???????:
> > 
> > I guess you can remove all the cable_plugged handling from s3c
> > battery driver, and then use power_supply.external_power_changed
> > callback to get cable_plugged notification (you should fill
> > pda_power.supplied_to properly to make it work).
> > 
> > See drivers/power/ds2760_battery.c and arch/arm/mach-pxa/hx4700.c
> > as an example.
> > 
> > You probably will need to change .external_power_changed() hook
> > to accept 'pst' argument (the supply that caused the change),
> > so that you could write something like this:
> > 
> > s3c_external_power_changed(struct power_supply *psy,
> > 			   struct power_supply *ext)
> > {
> > 	...
> > 	ext->get_property(ext, ONLINE, &val);
> > 	s3c->is_plugged = val.intval;
> > 
> > 	s3c_kick_cable_plugged_handler(s3c);
> > }
> > 
> > Or, instead of ext->get_property() you could just use
> > power_supply_is_system_supplied(). Not very elegant, but
> > should work.
> 
[...reordered...]
> pda_power driver does not support 'battery_charged' pin handling, but I need 
> to handle this pin (and IRQ from it) to prevent battery overcharge

And I was not talking about 'battery_charged' pin handling. :-)
Sure, battery_charged handling should be in the battery driver.

I was talking about 'cable_plugged' handling, it is the only
thing that s3c_adc_ac_get_property() is using, see the driver
you've just posted:

+static int s3c_adc_ac_get_property(struct power_supply *psy,
+                               enum power_supply_property psp,
+                               union power_supply_propval *val)
+{
+       int ret = 0;
+
+       switch (psp) {
+       case POWER_SUPPLY_PROP_ONLINE:
+               val->intval =
+                       !gpio_get_value(main_bat.pdata->gpio_cable_plugged);
+               break;
+       default:
+               ret = -EINVAL;
+               break;
+       }
+       return ret;
+}

And cable_plugged_handler() stays in the battery driver, but it
will be called by the pda_power driver via external_power_changed
callback.

> You didn't get the point. Here's workflow:
> cable plugged -> GPF2 goes to 0, irq is generated

Handled by the pda_power driver.

> cable plugged irq handler -> cable_plugged ? enable_charger : disable_charger

pda_power driver calls power_supply_changed(), which calls
s3c battery.external_power_changed/cable_plugged_handler,
which starts charging (sets is_plugged if
power_supply_is_system_supplied() returns true).

> charging....
> battery is charged, GPF3 goes to 0, irq is generated

Yep, this is handled by the s3c driver.

> battery charged irq handler -> disable_charger

This is still s3c driver's job.

> cable unplugged -> GPF2 goest to 1, irq is generated

Handled by the pda_power driver, it again sends the 'changed'
event to s3c battery driver, and the battery driver
clears is_plugged if power_supply_is_system_supplied() returns
false.

Still think this wont work?

-- 
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2

[PATCH 2/3] Add s3c-adc-battery driver

[Vasily Khoruzhick]

? ????????? ?? 9 ???? 2010 18:15:35 ????? Anton Vorontsov ???????:
> Still think this wont work?

Ok, one more question: pda_power requests irq with cleared TRIGGER flags, but 
I need TRIGGER_RISING | TRIGGER_FALLING, how can I add my custom irq flags to 
pda_power driver? :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100709/1d9bbe63/attachment.sig>

[PATCH 2/3] Add s3c-adc-battery driver

[Vasily Khoruzhick]

? ????????? ?? 9 ???? 2010 18:32:02 ????? Vasily Khoruzhick ???????:
> Ok, one more question: pda_power requests irq with cleared TRIGGER flags,
> but I need TRIGGER_RISING | TRIGGER_FALLING, how can I add my custom irq
> flags to pda_power driver? :)

Oops, sorry, they aren't cleared :) Sorry for the noise
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100709/130da2b9/attachment.sig>