Skip to contentSkip to conversation

Add Dialog DA9030 battery charger driver

RFC PATCH In review Mike Rapoport 4 participants 10 messages Dec 17, 2008 – Dec 30, 2008

lore
Share & tools
lore
#0 lore raw

RFC PATCH 2/2 Add Dialog DA9030 battery charger driver

Driver for battery charger integrated into Dialog Semiconductor DA9030 PMIC

 drivers/power/Kconfig          |    7 +
 drivers/power/Makefile         |    1 +
 drivers/power/da9030_battery.c |  592 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 600 insertions(+), 0 deletions(-)
drivers/power/Kconfig +7 -0
index 8e0c2b4..db8145c 100644
@@ -68,4 +68,11 @@ config BATTERY_BQ27x00
 	help
 	  Say Y here to enable support for batteries with BQ27200(I2C) chip.
 
+config BATTERY_DA903X
+	tristate "DA903x battery driver"
+	depends on PMIC_DA903X
+	help
+	  Say Y here to enable support for batteries charger integrated into
+	  DA9030/DA9030 PMICs.
+
 endif # POWER_SUPPLY
drivers/power/Makefile +1 -0
index e8f1ece..58d1b46 100644
@@ -23,3 +23,4 @@ obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
 obj-$(CONFIG_BATTERY_TOSA)	+= tosa_battery.o
 obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
 obj-$(CONFIG_BATTERY_BQ27x00)	+= bq27x00_battery.o
+obj-$(CONFIG_BATTERY_DA903X)	+= da903x_battery.o
drivers/power/da9030_battery.c new file +592 -0

592 lines not on this page · raw message

Signature · 2 lines
Sincerely yours,
Mike.
#1 lore raw
Quoted 52 lines · On Wed, 17 Dec 2008 09:56:10 +0200 Mike Rapoport <[email protected]> wrote:

Driver for battery charger integrated into Dialog Semiconductor DA9030 PMIC

 drivers/power/Kconfig          |    7 +
 drivers/power/Makefile         |    1 +
 drivers/power/da9030_battery.c |  592 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 600 insertions(+), 0 deletions(-)
drivers/power/Kconfig +7 -0
index 8e0c2b4..db8145c 100644
@@ -68,4 +68,11 @@ config BATTERY_BQ27x00
 	help
 	  Say Y here to enable support for batteries with BQ27200(I2C) chip.
 
+config BATTERY_DA903X
+	tristate "DA903x battery driver"
+	depends on PMIC_DA903X
+	help
+	  Say Y here to enable support for batteries charger integrated into
+	  DA9030/DA9030 PMICs.
+
 endif # POWER_SUPPLY
drivers/power/Makefile +1 -0
index e8f1ece..58d1b46 100644
@@ -23,3 +23,4 @@ obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
 obj-$(CONFIG_BATTERY_TOSA)	+= tosa_battery.o
 obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
 obj-$(CONFIG_BATTERY_BQ27x00)	+= bq27x00_battery.o
+obj-$(CONFIG_BATTERY_DA903X)	+= da903x_battery.o
drivers/power/da9030_battery.c new file +10 -0
new file mode 100644
index 0000000..2dd9fef
@@ -0,0 +1,592 @@
+/*
+ * Battery charger driver for Dialog Semiconductor DA9030
+ *
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <[email protected]>
+ *
+ * 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.
+ */

drivers/power/da9030_battery.c line 20

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/mfd/da903x.h>
+
+#ifdef CONFIG_DEBUG_FS
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#endif

It's not a good ideas to put these includes inside an ifdef. What happens is that later someones adds some seq_file stuff outside CONFIG_DEBUG_FS and it all works nicely so it gets merged. Then someone else disables debugfs and boom.

Quoted 40 lines
drivers/power/da9030_battery.c new file +40 -0
+#define DA9030_STATUS_CHDET	(1 << 3)
+
+#define DA9030_FAULT_LOG		0x0a
+#define DA9030_FAULT_LOG_OVER_TEMP	(1 << 7)
+#define DA9030_FAULT_LOG_VBAT_OVER	(1 << 4)
+
+#define DA9030_CHARGE_CONTROL		0x28
+#define DA9030_CHRG_CHARGER_ENABLE	(1 << 7)
+
+#define DA9030_ADC_MAN_CONTROL		0x30
+#define DA9030_ADC_TBATREF_ENABLE	(1 << 5)
+#define DA9030_ADC_LDO_INT_ENABLE	(1 << 4)
+
+#define DA9030_ADC_AUTO_CONTROL		0x31
+#define DA9030_ADC_TBAT_ENABLE		(1 << 5)
+#define DA9030_ADC_VBAT_IN_TXON		(1 << 4)
+#define DA9030_ADC_VCH_ENABLE		(1 << 3)
+#define DA9030_ADC_ICH_ENABLE		(1 << 2)
+#define DA9030_ADC_VBAT_ENABLE		(1 << 1)
+#define DA9030_ADC_AUTO_SLEEP_ENABLE	(1 << 0)
+
+#define DA9030_VBATMON		0x32
+#define DA9030_VBATMONTXON	0x33
+#define DA9030_TBATHIGHP	0x34
+#define DA9030_TBATHIGHN	0x35
+#define DA9030_TBATLOW		0x36
+
+#define DA9030_VBAT_RES		0x41
+#define DA9030_VBATMIN_RES	0x42
+#define DA9030_VBATMINTXON_RES	0x43
+#define DA9030_ICHMAX_RES	0x44
+#define DA9030_ICHMIN_RES	0x45
+#define DA9030_ICHAVERAGE_RES	0x46
+#define DA9030_VCHMAX_RES	0x47
+#define DA9030_VCHMIN_RES	0x48
+#define DA9030_TBAT_RES		0x49
+
+struct da9030_adc_res {
+	uint8_t vbat_res;
+	uint8_t vbatmin_res;

drivers/power/da9030_battery.c line 71

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	uint8_t vbatmintxon;
+	uint8_t ichmax_res;
+	uint8_t ichmin_res;
+	uint8_t ichaverage_res;
+	uint8_t vchmax_res;
+	uint8_t vchmin_res;
+	uint8_t tbat_res;
+	uint8_t adc_in4_res;
+	uint8_t adc_in5_res;
+};

hm. Are all the above fields sufficiently obvious as to not need any description?

Quoted 22 lines
drivers/power/da9030_battery.c new file +19 -0
+struct da9030_battery_thresholds {
+	int tbat_low;
+	int tbat_high;
+	int tbat_restart;
+
+	int vbat_low;
+	int vbat_crit;
+	int vbat_charge_start;
+	int vbat_charge_stop;
+	int vbat_charge_restart;
+
+	int vcharge_min;
+	int vcharge_max;
+};
+
 
...
 
+static struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)
+{
+	charger->debug_file = debugfs_create_file("charger", 0666, 0, charger,
+						 &bat_debug_fops);

drivers/power/da9030_battery.c

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	return charger->debug_file;
+}
+
+static void da9030_bat_remove_debugfs(struct da9030_charger *charger)
+{
+	debugfs_remove(charger->debug_file);
+}
+#else
+#define da9030_bat_create_debugfs(x)	NULL
+#define da9030_bat_remove_debugfs(x)	do {} while (0)

It would be better to make these stubs regular C functions, please. It's more symmetrical, more pleasing to the eye and provides typechecking when CONFIG_DEBUG_FS=n.

drivers/power/da9030_battery.c line 592

Quoted 8 lines
drivers/power/da9030_battery.c new file +5 -0
+#endif
+
 
...
 
+MODULE_DESCRIPTION("DA9030 battery charger driver");
+MODULE_AUTHOR("Mike Rapoport, CompuLab");
+MODULE_LICENSE("GPL");

A neat looking driver. You deprive me of nits to pick.

#2 lore raw
In reply to Mike Rapoport

Hi Mike,

Quoted 1 line · Mike Rapoport

Driver for battery charger integrated into Dialog Semiconductor DA9030 PMIC

I think it'd make sense for me to take this one into the MFD tree, right ? 2 remarks though:
1) I'd like to get Anton Ack on it.
2) This patch wont build, as the Makefile is trying to build da903x_battery.o while da9030_battery.c is created. I guess it'd make sense to rename it da903x_battery.c.

Cheers,
Samuel.

Quoted 635 lines
 drivers/power/Kconfig          |    7 +
 drivers/power/Makefile         |    1 +
 drivers/power/da9030_battery.c |  592 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 600 insertions(+), 0 deletions(-)
drivers/power/Kconfig +7 -0
index 8e0c2b4..db8145c 100644
@@ -68,4 +68,11 @@ config BATTERY_BQ27x00
 	help
 	  Say Y here to enable support for batteries with BQ27200(I2C) chip.
 
+config BATTERY_DA903X
+	tristate "DA903x battery driver"
+	depends on PMIC_DA903X
+	help
+	  Say Y here to enable support for batteries charger integrated into
+	  DA9030/DA9030 PMICs.
+
 endif # POWER_SUPPLY
drivers/power/Makefile +1 -0
index e8f1ece..58d1b46 100644
@@ -23,3 +23,4 @@ obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
 obj-$(CONFIG_BATTERY_TOSA)	+= tosa_battery.o
 obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
 obj-$(CONFIG_BATTERY_BQ27x00)	+= bq27x00_battery.o
+obj-$(CONFIG_BATTERY_DA903X)	+= da903x_battery.o
drivers/power/da9030_battery.c new file +592 -0
new file mode 100644
index 0000000..2dd9fef
@@ -0,0 +1,592 @@
+/*
+ * Battery charger driver for Dialog Semiconductor DA9030
+ *
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <[email protected]>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/mfd/da903x.h>
+
+#ifdef CONFIG_DEBUG_FS
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#endif
+
+#define DA9030_STATUS_CHDET	(1 << 3)
+
+#define DA9030_FAULT_LOG		0x0a
+#define DA9030_FAULT_LOG_OVER_TEMP	(1 << 7)
+#define DA9030_FAULT_LOG_VBAT_OVER	(1 << 4)
+
+#define DA9030_CHARGE_CONTROL		0x28
+#define DA9030_CHRG_CHARGER_ENABLE	(1 << 7)
+
+#define DA9030_ADC_MAN_CONTROL		0x30
+#define DA9030_ADC_TBATREF_ENABLE	(1 << 5)
+#define DA9030_ADC_LDO_INT_ENABLE	(1 << 4)
+
+#define DA9030_ADC_AUTO_CONTROL		0x31
+#define DA9030_ADC_TBAT_ENABLE		(1 << 5)
+#define DA9030_ADC_VBAT_IN_TXON		(1 << 4)
+#define DA9030_ADC_VCH_ENABLE		(1 << 3)
+#define DA9030_ADC_ICH_ENABLE		(1 << 2)
+#define DA9030_ADC_VBAT_ENABLE		(1 << 1)
+#define DA9030_ADC_AUTO_SLEEP_ENABLE	(1 << 0)
+
+#define DA9030_VBATMON		0x32
+#define DA9030_VBATMONTXON	0x33
+#define DA9030_TBATHIGHP	0x34
+#define DA9030_TBATHIGHN	0x35
+#define DA9030_TBATLOW		0x36
+
+#define DA9030_VBAT_RES		0x41
+#define DA9030_VBATMIN_RES	0x42
+#define DA9030_VBATMINTXON_RES	0x43
+#define DA9030_ICHMAX_RES	0x44
+#define DA9030_ICHMIN_RES	0x45
+#define DA9030_ICHAVERAGE_RES	0x46
+#define DA9030_VCHMAX_RES	0x47
+#define DA9030_VCHMIN_RES	0x48
+#define DA9030_TBAT_RES		0x49
+
+struct da9030_adc_res {
+	uint8_t vbat_res;
+	uint8_t vbatmin_res;
+	uint8_t vbatmintxon;
+	uint8_t ichmax_res;
+	uint8_t ichmin_res;
+	uint8_t ichaverage_res;
+	uint8_t vchmax_res;
+	uint8_t vchmin_res;
+	uint8_t tbat_res;
+	uint8_t adc_in4_res;
+	uint8_t adc_in5_res;
+};
+
+struct da9030_battery_thresholds {
+	int tbat_low;
+	int tbat_high;
+	int tbat_restart;
+
+	int vbat_low;
+	int vbat_crit;
+	int vbat_charge_start;
+	int vbat_charge_stop;
+	int vbat_charge_restart;
+
+	int vcharge_min;
+	int vcharge_max;
+};
+
+struct da9030_charger {
+	struct power_supply psy;
+
+	struct device *master;
+
+	struct da9030_adc_res adc;
+	struct delayed_work work;
+	unsigned int interval;
+
+	struct power_supply_info *battery_info;
+
+	struct da9030_battery_thresholds thresholds;
+
+	unsigned int charge_milliamp;
+	unsigned int charge_millivolt;
+
+	/* charger status */
+	int chdet:1;
+	uint8_t fault;
+	int mA;
+	int mV;
+	int is_on;
+
+	struct notifier_block nb;
+
+	/* platform callbacks for battery low and critical events */
+	void (*battery_low)(void);
+	void (*battery_critical)(void);
+
+#ifdef CONFIG_DEBUG_FS
+	struct dentry *debug_file;
+#endif
+};
+
+#ifdef CONFIG_DEBUG_FS
+
+static inline int da9030_reg_to_mV(int reg)
+{
+	return ((reg * 2650) >> 8) + 2650;
+}
+
+static inline int da9030_millivolt_to_reg(int mV)
+{
+	return ((mV - 2650) << 8) / 2650;
+}
+
+static inline int da9030_reg_to_mA(int reg)
+{
+	return ((reg * 24000) >> 8) / 15;
+}
+
+static int bat_debug_show(struct seq_file *s, void *data)
+{
+	struct da9030_charger *charger = s->private;
+
+	seq_printf(s, "charger is %s\n", charger->is_on ? "on" : "off");
+	if (charger->chdet) {
+		seq_printf(s, "iset = %dmA, vset = %dmV\n",
+			   charger->mA, charger->mV);
+	}
+
+	seq_printf(s, "vbat_res = %d (%dmV)\n",
+		   charger->adc.vbat_res,
+		   da9030_reg_to_mV(charger->adc.vbat_res));
+	seq_printf(s, "vbatmin_res = %d (%dmV)\n",
+		   charger->adc.vbatmin_res,
+		   da9030_reg_to_mV(charger->adc.vbatmin_res));
+	seq_printf(s, "vbatmintxon = %d (%dmV)\n",
+		   charger->adc.vbatmintxon,
+		   da9030_reg_to_mV(charger->adc.vbatmintxon));
+	seq_printf(s, "ichmax_res = %d (%dmA)\n",
+		   charger->adc.ichmax_res,
+		   da9030_reg_to_mV(charger->adc.ichmax_res));
+	seq_printf(s, "ichmin_res = %d (%dmA)\n",
+		   charger->adc.ichmin_res,
+		   da9030_reg_to_mA(charger->adc.ichmin_res));
+	seq_printf(s, "ichaverage_res = %d (%dmA)\n",
+		   charger->adc.ichaverage_res,
+		   da9030_reg_to_mA(charger->adc.ichaverage_res));
+	seq_printf(s, "vchmax_res = %d (%dmV)\n",
+		   charger->adc.vchmax_res,
+		   da9030_reg_to_mA(charger->adc.vchmax_res));
+	seq_printf(s, "vchmin_res = %d (%dmV)\n",
+		   charger->adc.vchmin_res,
+		   da9030_reg_to_mV(charger->adc.vchmin_res));
+
+	return 0;
+}
+
+static int debug_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, bat_debug_show, inode->i_private);
+}
+
+static const struct file_operations bat_debug_fops = {
+	.open		= debug_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)
+{
+	charger->debug_file = debugfs_create_file("charger", 0666, 0, charger,
+						 &bat_debug_fops);
+	return charger->debug_file;
+}
+
+static void da9030_bat_remove_debugfs(struct da9030_charger *charger)
+{
+	debugfs_remove(charger->debug_file);
+}
+#else
+#define da9030_bat_create_debugfs(x)	NULL
+#define da9030_bat_remove_debugfs(x)	do {} while (0)
+#endif
+
+static inline void da9030_read_adc(struct da9030_charger *charger,
+				   struct da9030_adc_res *adc)
+{
+	da903x_reads(charger->master, DA9030_VBAT_RES,
+		     sizeof(*adc), (uint8_t *)adc);
+}
+
+static void da9030_charger_update_state(struct da9030_charger *charger)
+{
+	uint8_t val;
+
+	da903x_read(charger->master, DA9030_CHARGE_CONTROL, &val);
+	charger->is_on = (val & DA9030_CHRG_CHARGER_ENABLE) ? 1 : 0;
+	charger->mA = ((val >> 3) & 0xf) * 100;
+	charger->mV = (val & 0x7) * 50 + 4000;
+
+	da9030_read_adc(charger, &charger->adc);
+	da903x_read(charger->master, DA9030_FAULT_LOG, &charger->fault);
+	charger->chdet = da903x_query_status(charger->master,
+						     DA9030_STATUS_CHDET);
+}
+
+static void da9030_set_charge(struct da9030_charger *charger, int on)
+{
+	uint8_t val = 0;
+
+	if (on) {
+		val = DA9030_CHRG_CHARGER_ENABLE;
+		val |= (charger->charge_milliamp / 100) << 3;
+		val |= (charger->charge_millivolt - 4000) / 50;
+		charger->is_on = 1;
+	} else {
+		val = 0;
+		charger->is_on = 0;
+	}
+
+	da903x_write(charger->master, DA9030_CHARGE_CONTROL, val);
+}
+
+static void da9030_charger_check_state(struct da9030_charger *charger)
+{
+	da9030_charger_update_state(charger);
+
+	/* we wake or boot with external power on */
+	if (!charger->is_on) {
+		if ((charger->chdet) &&
+		    (charger->adc.vbat_res <
+		     charger->thresholds.vbat_charge_start)) {
+			da9030_set_charge(charger, 1);
+		}
+	} else {
+		if (charger->adc.vbat_res >=
+		    charger->thresholds.vbat_charge_stop) {
+			da9030_set_charge(charger, 0);
+			da903x_write(charger->master, DA9030_VBATMON,
+				       charger->thresholds.vbat_charge_restart);
+		} else if (charger->adc.vbat_res >
+			   charger->thresholds.vbat_low) {
+			/* we are charging and passed LOW_THRESH,
+			   so upate DA9030 VBAT threshold
+			 */
+			da903x_write(charger->master, DA9030_VBATMON,
+				     charger->thresholds.vbat_low);
+		}
+		if (charger->adc.vchmax_res > charger->thresholds.vcharge_max ||
+		    charger->adc.vchmin_res < charger->thresholds.vcharge_min ||
+		    /* Tempreture readings are negative */
+		    charger->adc.tbat_res < charger->thresholds.tbat_high ||
+		    charger->adc.tbat_res > charger->thresholds.tbat_low) {
+			/* disable charger */
+			da9030_set_charge(charger, 0);
+		}
+	}
+}
+
+static void da9030_charging_monitor(struct work_struct *work)
+{
+	struct da9030_charger *charger;
+
+	charger = container_of(work, struct da9030_charger, work.work);
+
+	da9030_charger_check_state(charger);
+
+	/* reschedule for the next time */
+	schedule_delayed_work(&charger->work, charger->interval);
+}
+
+static enum power_supply_property da9030_battery_props[] = {
+	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_AVG,
+};
+
+static void da9030_battery_check_status(struct da9030_charger *charger,
+				    union power_supply_propval *val)
+{
+	if (charger->chdet) {
+		if (charger->is_on)
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+		else
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+	} else {
+		val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+	}
+}
+
+static void da9030_battery_check_health(struct da9030_charger *charger,
+				    union power_supply_propval *val)
+{
+	if (charger->fault & DA9030_FAULT_LOG_OVER_TEMP)
+		val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+	else if (charger->fault & DA9030_FAULT_LOG_VBAT_OVER)
+		val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+	else
+		val->intval = POWER_SUPPLY_HEALTH_GOOD;
+}
+
+static int da9030_battery_get_property(struct power_supply *psy,
+				   enum power_supply_property psp,
+				   union power_supply_propval *val)
+{
+	struct da9030_charger *charger;
+	charger = container_of(psy, struct da9030_charger, psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		da9030_battery_check_status(charger, val);
+		break;
+	case POWER_SUPPLY_PROP_HEALTH:
+		da9030_battery_check_health(charger, val);
+		break;
+	case POWER_SUPPLY_PROP_TECHNOLOGY:
+		val->intval = charger->battery_info->technology;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		val->intval = charger->battery_info->voltage_max_design;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		val->intval = charger->battery_info->voltage_min_design;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = da9030_reg_to_mV(charger->adc.vbat_res) * 1000;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_AVG:
+		val->intval =
+			da9030_reg_to_mA(charger->adc.ichaverage_res) * 1000;
+		break;
+	case POWER_SUPPLY_PROP_MODEL_NAME:
+		val->strval = charger->battery_info->name;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static void da9030_battery_vbat_event(struct da9030_charger *charger)
+{
+	da9030_read_adc(charger, &charger->adc);
+
+	if (!charger->is_on) {
+		if (charger->adc.vbat_res < charger->thresholds.vbat_low) {
+			/* set VBAT threshold for critical */
+			da903x_write(charger->master, DA9030_VBATMON,
+				     charger->thresholds.vbat_crit);
+			if (charger->battery_low)
+				charger->battery_low();
+		} else if (charger->adc.vbat_res <
+			   charger->thresholds.vbat_crit) {
+			/* notify the system of battery critical */
+			if (charger->battery_critical)
+				charger->battery_critical();
+		}
+	}
+}
+
+static int da9030_battery_event(struct notifier_block *nb, unsigned long event,
+				void *data)
+{
+	struct da9030_charger *charger =
+		container_of(nb, struct da9030_charger, nb);
+	int status;
+
+	switch (event) {
+	case DA9030_EVENT_CHDET:
+		status = da903x_query_status(charger->master,
+					     DA9030_STATUS_CHDET);
+		da9030_set_charge(charger, status);
+		break;
+	case DA9030_EVENT_VBATMON:
+		da9030_battery_vbat_event(charger);
+		break;
+	case DA9030_EVENT_CHIOVER:
+	case DA9030_EVENT_TBAT:
+		da9030_set_charge(charger, 0);
+		break;
+	}
+
+	return 0;
+}
+
+static void da9030_battery_convert_thresholds(struct da9030_charger *charger,
+					      struct da9030_battery_info *pdata)
+{
+	charger->thresholds.tbat_low = pdata->tbat_low;
+	charger->thresholds.tbat_high = pdata->tbat_high;
+	charger->thresholds.tbat_restart  = pdata->tbat_restart;
+
+	charger->thresholds.vbat_low =
+		da9030_millivolt_to_reg(pdata->vbat_low);
+	charger->thresholds.vbat_crit =
+		da9030_millivolt_to_reg(pdata->vbat_crit);
+	charger->thresholds.vbat_charge_start =
+		da9030_millivolt_to_reg(pdata->vbat_charge_start);
+	charger->thresholds.vbat_charge_stop =
+		da9030_millivolt_to_reg(pdata->vbat_charge_stop);
+	charger->thresholds.vbat_charge_restart =
+		da9030_millivolt_to_reg(pdata->vbat_charge_restart);
+
+	charger->thresholds.vcharge_min =
+		da9030_millivolt_to_reg(pdata->vcharge_min);
+	charger->thresholds.vcharge_max =
+		da9030_millivolt_to_reg(pdata->vcharge_max);
+}
+
+static void da9030_battery_setup_psy(struct da9030_charger *charger)
+{
+	struct power_supply *psy = &charger->psy;
+	struct power_supply_info *info = charger->battery_info;
+
+	psy->name = info->name;
+	psy->use_for_apm = info->use_for_apm;
+	psy->type = POWER_SUPPLY_TYPE_BATTERY;
+	psy->get_property = da9030_battery_get_property;
+
+	psy->properties = da9030_battery_props;
+	psy->num_properties = ARRAY_SIZE(da9030_battery_props);;
+};
+
+static int da9030_battery_charger_init(struct da9030_charger *charger)
+{
+	char v[5];
+	int ret;
+
+	v[0] = v[1] = charger->thresholds.vbat_low;
+	v[2] = charger->thresholds.tbat_high;
+	v[3] = charger->thresholds.tbat_restart;
+	v[4] = charger->thresholds.tbat_low;
+
+	ret = da903x_writes(charger->master, DA9030_VBATMON, 5, v);
+	if (ret)
+		return ret;
+
+	/* enable reference voltage supply for ADC from the LDO_INTERNAL
+	   regulator. Must be set before ADC measurements can be made. */
+	ret = da903x_write(charger->master, DA9030_ADC_MAN_CONTROL,
+			   DA9030_ADC_LDO_INT_ENABLE |
+			   DA9030_ADC_TBATREF_ENABLE);
+	if (ret)
+		return ret;
+
+	/* enable auto ADC measuremnts */
+	return da903x_write(charger->master, DA9030_ADC_AUTO_CONTROL,
+			    DA9030_ADC_TBAT_ENABLE | DA9030_ADC_VBAT_IN_TXON |
+			    DA9030_ADC_VCH_ENABLE | DA9030_ADC_ICH_ENABLE |
+			    DA9030_ADC_VBAT_ENABLE |
+			    DA9030_ADC_AUTO_SLEEP_ENABLE);
+}
+
+static int da9030_battery_probe(struct platform_device *pdev)
+{
+	struct da9030_charger *charger;
+	struct da9030_battery_info *pdata = pdev->dev.platform_data;
+	int ret;
+
+	if (pdata == NULL)
+		return -EINVAL;
+
+	if (pdata->charge_milliamp >= 1500 ||
+	    pdata->charge_millivolt < 4000 ||
+	    pdata->charge_millivolt > 4350)
+		return -EINVAL;
+
+	charger = kzalloc(sizeof(*charger), GFP_KERNEL);
+	if (charger == NULL)
+		return -ENOMEM;
+
+	charger->master = pdev->dev.parent;
+
+	/* 10 seconds between monotor runs unless platfrom defines other
+	   interval */
+	charger->interval = msecs_to_jiffies(
+		(pdata->batmon_interval ? : 10) * 1000);
+
+	charger->charge_milliamp = pdata->charge_milliamp;
+	charger->charge_millivolt = pdata->charge_millivolt;
+	charger->battery_info = pdata->battery_info;
+	charger->battery_low = pdata->battery_low;
+	charger->battery_critical = pdata->battery_critical;
+
+	da9030_battery_convert_thresholds(charger, pdata);
+
+	ret = da9030_battery_charger_init(charger);
+	if (ret)
+		goto err_charger_init;
+
+	INIT_DELAYED_WORK(&charger->work, da9030_charging_monitor);
+	schedule_delayed_work(&charger->work, charger->interval);
+
+	charger->nb.notifier_call = da9030_battery_event;
+	ret = da903x_register_notifier(charger->master, &charger->nb,
+				       DA9030_EVENT_CHDET |
+				       DA9030_EVENT_VBATMON |
+				       DA9030_EVENT_CHIOVER |
+				       DA9030_EVENT_TBAT);
+	if (ret)
+		goto err_notifier;
+
+	da9030_battery_setup_psy(charger);
+	ret = power_supply_register(&pdev->dev, &charger->psy);
+	if (ret)
+		goto err_ps_register;
+
+	charger->debug_file = da9030_bat_create_debugfs(charger);
+	platform_set_drvdata(pdev, charger);
+	return 0;
+
+err_ps_register:
+	da903x_unregister_notifier(charger->master, &charger->nb,
+				   DA9030_EVENT_CHDET | DA9030_EVENT_VBATMON |
+				   DA9030_EVENT_CHIOVER | DA9030_EVENT_TBAT);
+err_notifier:
+	cancel_delayed_work(&charger->work);
+
+err_charger_init:
+	kfree(charger);
+
+	return ret;
+}
+
+static int da9030_battery_remove(struct platform_device *dev)
+{
+	struct da9030_charger *charger = platform_get_drvdata(dev);
+
+	da9030_bat_remove_debugfs(charger);
+
+	da903x_unregister_notifier(charger->master, &charger->nb,
+				   DA9030_EVENT_CHDET | DA9030_EVENT_VBATMON |
+				   DA9030_EVENT_CHIOVER | DA9030_EVENT_TBAT);
+	cancel_delayed_work(&charger->work);
+	power_supply_unregister(&charger->psy);
+
+	kfree(charger);
+
+	return 0;
+}
+
+static struct platform_driver da903x_battery_driver = {
+	.driver	= {
+		.name	= "da903x-battery",
+		.owner	= THIS_MODULE,
+	},
+	.probe = da9030_battery_probe,
+	.remove = da9030_battery_remove,
+};
+
+static int da903x_battery_init(void)
+{
+	return platform_driver_register(&da903x_battery_driver);
+}
+
+static void da903x_battery_exit(void)
+{
+	platform_driver_unregister(&da903x_battery_driver);
+}
+
+module_init(da903x_battery_init);
+module_exit(da903x_battery_exit);
+
+MODULE_DESCRIPTION("DA9030 battery charger driver");
+MODULE_AUTHOR("Mike Rapoport, CompuLab");
+MODULE_LICENSE("GPL");
Signature · 3 lines
-- 
Sincerely yours,
Mike.
Signature · 2 lines
Intel Open Source Technology Centre
http://oss.intel.com/
#3 lore raw

Hi Samuel,

Quoted 5 lines · Samuel Ortiz

Hi Mike,

Quoted 1 line · Mike Rapoport

Driver for battery charger integrated into Dialog Semiconductor DA9030 PMIC

I think it'd make sense for me to take this one into the MFD tree, right ?

I thought it'll go through battery tree, but for now Andrew already merged it to the -mm tree. I have no preference here as long as driver gets merged :).

Quoted 5 lines

2 remarks though:
1) I'd like to get Anton Ack on it.
2) This patch wont build, as the Makefile is trying to build da903x_battery.o while da9030_battery.c is created. I guess it'd make sense to rename it da903x_battery.c.

Last minute changes...
I actually prefer to keep it da9030 for now because I have no idea how much DA9034 charger differs from DA9030.
So, here's the updated patch that fixes Kconfig and Makefile entries and addresses the points Andrew raised at [1]

--
[1] http://lkml.org/lkml/2008/12/18/20.

Quoted 3 lines

Cheers,
Samuel.

 drivers/power/Kconfig          |    7 +
 drivers/power/Makefile         |    1 +
 drivers/power/da9030_battery.c |  593 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 601 insertions(+), 0 deletions(-)
drivers/power/Kconfig +7 -0
index 8e0c2b4..afa17b9 100644
@@ -68,4 +68,11 @@ config BATTERY_BQ27x00
 	help
 	  Say Y here to enable support for batteries with BQ27200(I2C) chip.
 
+config BATTERY_DA9030
+	tristate "DA9030 battery driver"
+	depends on PMIC_DA903X
+	help
+	  Say Y here to enable support for batteries charger integrated into
+	  DA9030 PMIC.
+
 endif # POWER_SUPPLY
drivers/power/Makefile +1 -0
index e8f1ece..1c83ede 100644
@@ -23,3 +23,4 @@ obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
 obj-$(CONFIG_BATTERY_TOSA)	+= tosa_battery.o
 obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
 obj-$(CONFIG_BATTERY_BQ27x00)	+= bq27x00_battery.o
+obj-$(CONFIG_BATTERY_DA9030)	+= da9030_battery.o
drivers/power/da9030_battery.c new file +593 -0

593 lines not on this page · raw message

Signature · 2 lines
Sincerely yours,
Mike.
#4 lore raw

Hi all,

On Thu, Dec 18, 2008 at 01:36:28PM +0200, Mike Rapoport wrote:
[...]

Quoted 2 lines
Quoted 2 lines

2 remarks though:
1) I'd like to get Anton Ack on it.

Samuel, as it depends on the MFD part, feel free to merge it into your tree.

Quoted 9 lines
Quoted 3 lines

2) This patch wont build, as the Makefile is trying to build da903x_battery.o while da9030_battery.c is created. I guess it'd make sense to rename it da903x_battery.c.

Last minute changes...
I actually prefer to keep it da9030 for now because I have no idea how much DA9034 charger differs from DA9030.
So, here's the updated patch that fixes Kconfig and Makefile entries and addresses the points Andrew raised at [1]

Mike,

Thanks for the patch, looks really good. Few comments down below.

[...]

drivers/power/da9030_battery.c line 11

Quoted 13 lines

+++ b/drivers/power/da9030_battery.c

drivers/power/da9030_battery.c new file +11 -0
@@ -0,0 +1,593 @@
+/*
+ * Battery charger driver for Dialog Semiconductor DA9030
+ *
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <[email protected]>
+ *
+ * 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.
+ */
+

The code should explicitly include all the needed headers.

#include <linux/kernel.h> (for container_of, sprintf, ...) #include <linux/init.h> (initcalls)
#include <linux/types.h> (uint8_t)
#include <linux/device.h> (struct device) #include <linux/workqueue.h> (delayedwork)

^^^ This is the only real comment. All the rest are nitpicks, which you can freely ignore if you like.

Quoted 82 lines
drivers/power/da9030_battery.c new file +82 -0
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/mfd/da903x.h>
+
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+
+#define DA9030_STATUS_CHDET	(1 << 3)
+
+#define DA9030_FAULT_LOG		0x0a
+#define DA9030_FAULT_LOG_OVER_TEMP	(1 << 7)
+#define DA9030_FAULT_LOG_VBAT_OVER	(1 << 4)
+
+#define DA9030_CHARGE_CONTROL		0x28
+#define DA9030_CHRG_CHARGER_ENABLE	(1 << 7)
+
+#define DA9030_ADC_MAN_CONTROL		0x30
+#define DA9030_ADC_TBATREF_ENABLE	(1 << 5)
+#define DA9030_ADC_LDO_INT_ENABLE	(1 << 4)
+
+#define DA9030_ADC_AUTO_CONTROL		0x31
+#define DA9030_ADC_TBAT_ENABLE		(1 << 5)
+#define DA9030_ADC_VBAT_IN_TXON		(1 << 4)
+#define DA9030_ADC_VCH_ENABLE		(1 << 3)
+#define DA9030_ADC_ICH_ENABLE		(1 << 2)
+#define DA9030_ADC_VBAT_ENABLE		(1 << 1)
+#define DA9030_ADC_AUTO_SLEEP_ENABLE	(1 << 0)
+
+#define DA9030_VBATMON		0x32
+#define DA9030_VBATMONTXON	0x33
+#define DA9030_TBATHIGHP	0x34
+#define DA9030_TBATHIGHN	0x35
+#define DA9030_TBATLOW		0x36
+
+#define DA9030_VBAT_RES		0x41
+#define DA9030_VBATMIN_RES	0x42
+#define DA9030_VBATMINTXON_RES	0x43
+#define DA9030_ICHMAX_RES	0x44
+#define DA9030_ICHMIN_RES	0x45
+#define DA9030_ICHAVERAGE_RES	0x46
+#define DA9030_VCHMAX_RES	0x47
+#define DA9030_VCHMIN_RES	0x48
+#define DA9030_TBAT_RES		0x49
+
+struct da9030_adc_res {
+	uint8_t vbat_res;
+	uint8_t vbatmin_res;
+	uint8_t vbatmintxon;
+	uint8_t ichmax_res;
+	uint8_t ichmin_res;
+	uint8_t ichaverage_res;
+	uint8_t vchmax_res;
+	uint8_t vchmin_res;
+	uint8_t tbat_res;
+	uint8_t adc_in4_res;
+	uint8_t adc_in5_res;
+};
+
+struct da9030_battery_thresholds {
+	int tbat_low;
+	int tbat_high;
+	int tbat_restart;
+
+	int vbat_low;
+	int vbat_crit;
+	int vbat_charge_start;
+	int vbat_charge_stop;
+	int vbat_charge_restart;
+
+	int vcharge_min;
+	int vcharge_max;
+};
+
+struct da9030_charger {
+	struct power_supply psy;
+
+	struct device *master;
+
+	struct da9030_adc_res adc;
+	struct delayed_work work;
+	unsigned int interval;

drivers/power/da9030_battery.c line 103

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+
+	struct power_supply_info *battery_info;
+
+	struct da9030_battery_thresholds thresholds;
+
+	unsigned int charge_milliamp;
+	unsigned int charge_millivolt;
+
+	/* charger status */
+	int chdet:1;

In kernel we can use bool type, and true/false constants.

Quoted 21 lines
drivers/power/da9030_battery.c new file +21 -0
+	uint8_t fault;
+	int mA;
+	int mV;
+	int is_on;
+
+	struct notifier_block nb;
+
+	/* platform callbacks for battery low and critical events */
+	void (*battery_low)(void);
+	void (*battery_critical)(void);
+
+	struct dentry *debug_file;
+};
+
+static inline int da9030_reg_to_mV(int reg)
+{
+	return ((reg * 2650) >> 8) + 2650;
+}
+
+static inline int da9030_millivolt_to_reg(int mV)
+{

drivers/power/da9030_battery.c line 134

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	return ((mV - 2650) << 8) / 2650;
+}
+
+static inline int da9030_reg_to_mA(int reg)
+{
+	return ((reg * 24000) >> 8) / 15;
+}
+
+#ifdef CONFIG_DEBUG_FS
+

(1)

Quoted 51 lines
drivers/power/da9030_battery.c new file +51 -0
+static int bat_debug_show(struct seq_file *s, void *data)
+{
+	struct da9030_charger *charger = s->private;
+
+	seq_printf(s, "charger is %s\n", charger->is_on ? "on" : "off");
+	if (charger->chdet) {
+		seq_printf(s, "iset = %dmA, vset = %dmV\n",
+			   charger->mA, charger->mV);
+	}
+
+	seq_printf(s, "vbat_res = %d (%dmV)\n",
+		   charger->adc.vbat_res,
+		   da9030_reg_to_mV(charger->adc.vbat_res));
+	seq_printf(s, "vbatmin_res = %d (%dmV)\n",
+		   charger->adc.vbatmin_res,
+		   da9030_reg_to_mV(charger->adc.vbatmin_res));
+	seq_printf(s, "vbatmintxon = %d (%dmV)\n",
+		   charger->adc.vbatmintxon,
+		   da9030_reg_to_mV(charger->adc.vbatmintxon));
+	seq_printf(s, "ichmax_res = %d (%dmA)\n",
+		   charger->adc.ichmax_res,
+		   da9030_reg_to_mV(charger->adc.ichmax_res));
+	seq_printf(s, "ichmin_res = %d (%dmA)\n",
+		   charger->adc.ichmin_res,
+		   da9030_reg_to_mA(charger->adc.ichmin_res));
+	seq_printf(s, "ichaverage_res = %d (%dmA)\n",
+		   charger->adc.ichaverage_res,
+		   da9030_reg_to_mA(charger->adc.ichaverage_res));
+	seq_printf(s, "vchmax_res = %d (%dmV)\n",
+		   charger->adc.vchmax_res,
+		   da9030_reg_to_mA(charger->adc.vchmax_res));
+	seq_printf(s, "vchmin_res = %d (%dmV)\n",
+		   charger->adc.vchmin_res,
+		   da9030_reg_to_mV(charger->adc.vchmin_res));
+
+	return 0;
+}
+
+static int debug_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, bat_debug_show, inode->i_private);
+}
+
+static const struct file_operations bat_debug_fops = {
+	.open		= debug_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)

drivers/power/da9030_battery.c line 195

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+{
+	charger->debug_file = debugfs_create_file("charger", 0666, 0, charger,
+						 &bat_debug_fops);
+	return charger->debug_file;
+}
+
+static void da9030_bat_remove_debugfs(struct da9030_charger *charger)
+{
+	debugfs_remove(charger->debug_file);
+}

I'd put an empty line here, or remove empty line at (1). Just for consistency.

Quoted 25 lines
drivers/power/da9030_battery.c new file +25 -0
+#else
+static inline struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)
+{
+	return NULL;
+}
+static inline void da9030_bat_remove_debugfs(struct da9030_charger *charger)
+{
+}
+#endif
+
+static inline void da9030_read_adc(struct da9030_charger *charger,
+				   struct da9030_adc_res *adc)
+{
+	da903x_reads(charger->master, DA9030_VBAT_RES,
+		     sizeof(*adc), (uint8_t *)adc);
+}
+
+static void da9030_charger_update_state(struct da9030_charger *charger)
+{
+	uint8_t val;
+
+	da903x_read(charger->master, DA9030_CHARGE_CONTROL, &val);
+	charger->is_on = (val & DA9030_CHRG_CHARGER_ENABLE) ? 1 : 0;
+	charger->mA = ((val >> 3) & 0xf) * 100;
+	charger->mV = (val & 0x7) * 50 + 4000;

drivers/power/da9030_battery.c line 230

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+
+	da9030_read_adc(charger, &charger->adc);
+	da903x_read(charger->master, DA9030_FAULT_LOG, &charger->fault);
+	charger->chdet = da903x_query_status(charger->master,
+						     DA9030_STATUS_CHDET);
+}
+
+static void da9030_set_charge(struct da9030_charger *charger, int on)
+{
+	uint8_t val = 0;

= 0 isn't necessary here.

Quoted 132 lines
drivers/power/da9030_battery.c new file +132 -0
+
+	if (on) {
+		val = DA9030_CHRG_CHARGER_ENABLE;
+		val |= (charger->charge_milliamp / 100) << 3;
+		val |= (charger->charge_millivolt - 4000) / 50;
+		charger->is_on = 1;
+	} else {
+		val = 0;
+		charger->is_on = 0;
+	}
+
+	da903x_write(charger->master, DA9030_CHARGE_CONTROL, val);
+}
+
+static void da9030_charger_check_state(struct da9030_charger *charger)
+{
+	da9030_charger_update_state(charger);
+
+	/* we wake or boot with external power on */
+	if (!charger->is_on) {
+		if ((charger->chdet) &&
+		    (charger->adc.vbat_res <
+		     charger->thresholds.vbat_charge_start)) {
+			da9030_set_charge(charger, 1);
+		}
+	} else {
+		if (charger->adc.vbat_res >=
+		    charger->thresholds.vbat_charge_stop) {
+			da9030_set_charge(charger, 0);
+			da903x_write(charger->master, DA9030_VBATMON,
+				       charger->thresholds.vbat_charge_restart);
+		} else if (charger->adc.vbat_res >
+			   charger->thresholds.vbat_low) {
+			/* we are charging and passed LOW_THRESH,
+			   so upate DA9030 VBAT threshold
+			 */
+			da903x_write(charger->master, DA9030_VBATMON,
+				     charger->thresholds.vbat_low);
+		}
+		if (charger->adc.vchmax_res > charger->thresholds.vcharge_max ||
+		    charger->adc.vchmin_res < charger->thresholds.vcharge_min ||
+		    /* Tempreture readings are negative */
+		    charger->adc.tbat_res < charger->thresholds.tbat_high ||
+		    charger->adc.tbat_res > charger->thresholds.tbat_low) {
+			/* disable charger */
+			da9030_set_charge(charger, 0);
+		}
+	}
+}
+
+static void da9030_charging_monitor(struct work_struct *work)
+{
+	struct da9030_charger *charger;
+
+	charger = container_of(work, struct da9030_charger, work.work);
+
+	da9030_charger_check_state(charger);
+
+	/* reschedule for the next time */
+	schedule_delayed_work(&charger->work, charger->interval);
+}
+
+static enum power_supply_property da9030_battery_props[] = {
+	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_AVG,
+};
+
+static void da9030_battery_check_status(struct da9030_charger *charger,
+				    union power_supply_propval *val)
+{
+	if (charger->chdet) {
+		if (charger->is_on)
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+		else
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+	} else {
+		val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+	}
+}
+
+static void da9030_battery_check_health(struct da9030_charger *charger,
+				    union power_supply_propval *val)
+{
+	if (charger->fault & DA9030_FAULT_LOG_OVER_TEMP)
+		val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+	else if (charger->fault & DA9030_FAULT_LOG_VBAT_OVER)
+		val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+	else
+		val->intval = POWER_SUPPLY_HEALTH_GOOD;
+}
+
+static int da9030_battery_get_property(struct power_supply *psy,
+				   enum power_supply_property psp,
+				   union power_supply_propval *val)
+{
+	struct da9030_charger *charger;
+	charger = container_of(psy, struct da9030_charger, psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		da9030_battery_check_status(charger, val);
+		break;
+	case POWER_SUPPLY_PROP_HEALTH:
+		da9030_battery_check_health(charger, val);
+		break;
+	case POWER_SUPPLY_PROP_TECHNOLOGY:
+		val->intval = charger->battery_info->technology;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		val->intval = charger->battery_info->voltage_max_design;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		val->intval = charger->battery_info->voltage_min_design;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = da9030_reg_to_mV(charger->adc.vbat_res) * 1000;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_AVG:
+		val->intval =
+			da9030_reg_to_mA(charger->adc.ichaverage_res) * 1000;
+		break;
+	case POWER_SUPPLY_PROP_MODEL_NAME:
+		val->strval = charger->battery_info->name;
+		break;
+	default:
+		break;

drivers/power/da9030_battery.c line 372

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	}
+
+	return 0;
+}
+
+static void da9030_battery_vbat_event(struct da9030_charger *charger)
+{
+	da9030_read_adc(charger, &charger->adc);
+
+	if (!charger->is_on) {

I'd write it as:

if (charger->is_on)

	return;

the-rest;

Saves one indent level.

Quoted 66 lines
drivers/power/da9030_battery.c new file +66 -0
+		if (charger->adc.vbat_res < charger->thresholds.vbat_low) {
+			/* set VBAT threshold for critical */
+			da903x_write(charger->master, DA9030_VBATMON,
+				     charger->thresholds.vbat_crit);
+			if (charger->battery_low)
+				charger->battery_low();
+		} else if (charger->adc.vbat_res <
+			   charger->thresholds.vbat_crit) {
+			/* notify the system of battery critical */
+			if (charger->battery_critical)
+				charger->battery_critical();
+		}
+	}
+}
+
+static int da9030_battery_event(struct notifier_block *nb, unsigned long event,
+				void *data)
+{
+	struct da9030_charger *charger =
+		container_of(nb, struct da9030_charger, nb);
+	int status;
+
+	switch (event) {
+	case DA9030_EVENT_CHDET:
+		status = da903x_query_status(charger->master,
+					     DA9030_STATUS_CHDET);
+		da9030_set_charge(charger, status);
+		break;
+	case DA9030_EVENT_VBATMON:
+		da9030_battery_vbat_event(charger);
+		break;
+	case DA9030_EVENT_CHIOVER:
+	case DA9030_EVENT_TBAT:
+		da9030_set_charge(charger, 0);
+		break;
+	}
+
+	return 0;
+}
+
+static void da9030_battery_convert_thresholds(struct da9030_charger *charger,
+					      struct da9030_battery_info *pdata)
+{
+	charger->thresholds.tbat_low = pdata->tbat_low;
+	charger->thresholds.tbat_high = pdata->tbat_high;
+	charger->thresholds.tbat_restart  = pdata->tbat_restart;
+
+	charger->thresholds.vbat_low =
+		da9030_millivolt_to_reg(pdata->vbat_low);
+	charger->thresholds.vbat_crit =
+		da9030_millivolt_to_reg(pdata->vbat_crit);
+	charger->thresholds.vbat_charge_start =
+		da9030_millivolt_to_reg(pdata->vbat_charge_start);
+	charger->thresholds.vbat_charge_stop =
+		da9030_millivolt_to_reg(pdata->vbat_charge_stop);
+	charger->thresholds.vbat_charge_restart =
+		da9030_millivolt_to_reg(pdata->vbat_charge_restart);
+
+	charger->thresholds.vcharge_min =
+		da9030_millivolt_to_reg(pdata->vcharge_min);
+	charger->thresholds.vcharge_max =
+		da9030_millivolt_to_reg(pdata->vcharge_max);
+}
+
+static void da9030_battery_setup_psy(struct da9030_charger *charger)
+{

drivers/power/da9030_battery.c line 448

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	struct power_supply *psy = &charger->psy;
+	struct power_supply_info *info = charger->battery_info;
+
+	psy->name = info->name;
+	psy->use_for_apm = info->use_for_apm;
+	psy->type = POWER_SUPPLY_TYPE_BATTERY;
+	psy->get_property = da9030_battery_get_property;
+
+	psy->properties = da9030_battery_props;
+	psy->num_properties = ARRAY_SIZE(da9030_battery_props);;

One semicolon is enough.

Quoted 8 lines
drivers/power/da9030_battery.c new file +8 -0
+};
+
+static int da9030_battery_charger_init(struct da9030_charger *charger)
+{
+	char v[5];
+	int ret;
+
+	v[0] = v[1] = charger->thresholds.vbat_low;

drivers/power/da9030_battery.c line 466

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	v[2] = charger->thresholds.tbat_high;
+	v[3] = charger->thresholds.tbat_restart;
+	v[4] = charger->thresholds.tbat_low;
+
+	ret = da903x_writes(charger->master, DA9030_VBATMON, 5, v);
+	if (ret)
+		return ret;
+
+	/* enable reference voltage supply for ADC from the LDO_INTERNAL
+	   regulator. Must be set before ADC measurements can be made. */

I would suggest using canonical-style comments:

/*
* Not a big deal though, I'm
* just nitpicking. ;-)
*/

Thanks,

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#5 lore raw
Quoted 11 lines · Anton Vorontsov

Hi all,

On Thu, Dec 18, 2008 at 01:36:28PM +0200, Mike Rapoport wrote:
[...]

Quoted 2 lines
Quoted 2 lines

2 remarks though:
1) I'd like to get Anton Ack on it.

Samuel, as it depends on the MFD part, feel free to merge it into your tree.

Thanks Anton, I just applied Mike's latest version of the patch.

Cheers,
Samuel.

Quoted 534 lines
Quoted 9 lines
Quoted 3 lines

2) This patch wont build, as the Makefile is trying to build da903x_battery.o while da9030_battery.c is created. I guess it'd make sense to rename it da903x_battery.c.

Last minute changes...
I actually prefer to keep it da9030 for now because I have no idea how much DA9034 charger differs from DA9030.
So, here's the updated patch that fixes Kconfig and Makefile entries and addresses the points Andrew raised at [1]

Mike,

Thanks for the patch, looks really good. Few comments down below.

[...]

drivers/power/da9030_battery.c line 11

Quoted 13 lines

+++ b/drivers/power/da9030_battery.c

drivers/power/da9030_battery.c new file +11 -0
@@ -0,0 +1,593 @@
+/*
+ * Battery charger driver for Dialog Semiconductor DA9030
+ *
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <[email protected]>
+ *
+ * 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.
+ */
+

The code should explicitly include all the needed headers.

#include <linux/kernel.h> (for container_of, sprintf, ...) #include <linux/init.h> (initcalls)
#include <linux/types.h> (uint8_t)
#include <linux/device.h> (struct device) #include <linux/workqueue.h> (delayedwork)

^^^ This is the only real comment. All the rest are nitpicks, which you can freely ignore if you like.

Quoted 82 lines
drivers/power/da9030_battery.c new file +82 -0
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/mfd/da903x.h>
+
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+
+#define DA9030_STATUS_CHDET	(1 << 3)
+
+#define DA9030_FAULT_LOG		0x0a
+#define DA9030_FAULT_LOG_OVER_TEMP	(1 << 7)
+#define DA9030_FAULT_LOG_VBAT_OVER	(1 << 4)
+
+#define DA9030_CHARGE_CONTROL		0x28
+#define DA9030_CHRG_CHARGER_ENABLE	(1 << 7)
+
+#define DA9030_ADC_MAN_CONTROL		0x30
+#define DA9030_ADC_TBATREF_ENABLE	(1 << 5)
+#define DA9030_ADC_LDO_INT_ENABLE	(1 << 4)
+
+#define DA9030_ADC_AUTO_CONTROL		0x31
+#define DA9030_ADC_TBAT_ENABLE		(1 << 5)
+#define DA9030_ADC_VBAT_IN_TXON		(1 << 4)
+#define DA9030_ADC_VCH_ENABLE		(1 << 3)
+#define DA9030_ADC_ICH_ENABLE		(1 << 2)
+#define DA9030_ADC_VBAT_ENABLE		(1 << 1)
+#define DA9030_ADC_AUTO_SLEEP_ENABLE	(1 << 0)
+
+#define DA9030_VBATMON		0x32
+#define DA9030_VBATMONTXON	0x33
+#define DA9030_TBATHIGHP	0x34
+#define DA9030_TBATHIGHN	0x35
+#define DA9030_TBATLOW		0x36
+
+#define DA9030_VBAT_RES		0x41
+#define DA9030_VBATMIN_RES	0x42
+#define DA9030_VBATMINTXON_RES	0x43
+#define DA9030_ICHMAX_RES	0x44
+#define DA9030_ICHMIN_RES	0x45
+#define DA9030_ICHAVERAGE_RES	0x46
+#define DA9030_VCHMAX_RES	0x47
+#define DA9030_VCHMIN_RES	0x48
+#define DA9030_TBAT_RES		0x49
+
+struct da9030_adc_res {
+	uint8_t vbat_res;
+	uint8_t vbatmin_res;
+	uint8_t vbatmintxon;
+	uint8_t ichmax_res;
+	uint8_t ichmin_res;
+	uint8_t ichaverage_res;
+	uint8_t vchmax_res;
+	uint8_t vchmin_res;
+	uint8_t tbat_res;
+	uint8_t adc_in4_res;
+	uint8_t adc_in5_res;
+};
+
+struct da9030_battery_thresholds {
+	int tbat_low;
+	int tbat_high;
+	int tbat_restart;
+
+	int vbat_low;
+	int vbat_crit;
+	int vbat_charge_start;
+	int vbat_charge_stop;
+	int vbat_charge_restart;
+
+	int vcharge_min;
+	int vcharge_max;
+};
+
+struct da9030_charger {
+	struct power_supply psy;
+
+	struct device *master;
+
+	struct da9030_adc_res adc;
+	struct delayed_work work;
+	unsigned int interval;

drivers/power/da9030_battery.c line 103

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+
+	struct power_supply_info *battery_info;
+
+	struct da9030_battery_thresholds thresholds;
+
+	unsigned int charge_milliamp;
+	unsigned int charge_millivolt;
+
+	/* charger status */
+	int chdet:1;

In kernel we can use bool type, and true/false constants.

Quoted 21 lines
drivers/power/da9030_battery.c new file +21 -0
+	uint8_t fault;
+	int mA;
+	int mV;
+	int is_on;
+
+	struct notifier_block nb;
+
+	/* platform callbacks for battery low and critical events */
+	void (*battery_low)(void);
+	void (*battery_critical)(void);
+
+	struct dentry *debug_file;
+};
+
+static inline int da9030_reg_to_mV(int reg)
+{
+	return ((reg * 2650) >> 8) + 2650;
+}
+
+static inline int da9030_millivolt_to_reg(int mV)
+{

drivers/power/da9030_battery.c line 134

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	return ((mV - 2650) << 8) / 2650;
+}
+
+static inline int da9030_reg_to_mA(int reg)
+{
+	return ((reg * 24000) >> 8) / 15;
+}
+
+#ifdef CONFIG_DEBUG_FS
+

(1)

Quoted 51 lines
drivers/power/da9030_battery.c new file +51 -0
+static int bat_debug_show(struct seq_file *s, void *data)
+{
+	struct da9030_charger *charger = s->private;
+
+	seq_printf(s, "charger is %s\n", charger->is_on ? "on" : "off");
+	if (charger->chdet) {
+		seq_printf(s, "iset = %dmA, vset = %dmV\n",
+			   charger->mA, charger->mV);
+	}
+
+	seq_printf(s, "vbat_res = %d (%dmV)\n",
+		   charger->adc.vbat_res,
+		   da9030_reg_to_mV(charger->adc.vbat_res));
+	seq_printf(s, "vbatmin_res = %d (%dmV)\n",
+		   charger->adc.vbatmin_res,
+		   da9030_reg_to_mV(charger->adc.vbatmin_res));
+	seq_printf(s, "vbatmintxon = %d (%dmV)\n",
+		   charger->adc.vbatmintxon,
+		   da9030_reg_to_mV(charger->adc.vbatmintxon));
+	seq_printf(s, "ichmax_res = %d (%dmA)\n",
+		   charger->adc.ichmax_res,
+		   da9030_reg_to_mV(charger->adc.ichmax_res));
+	seq_printf(s, "ichmin_res = %d (%dmA)\n",
+		   charger->adc.ichmin_res,
+		   da9030_reg_to_mA(charger->adc.ichmin_res));
+	seq_printf(s, "ichaverage_res = %d (%dmA)\n",
+		   charger->adc.ichaverage_res,
+		   da9030_reg_to_mA(charger->adc.ichaverage_res));
+	seq_printf(s, "vchmax_res = %d (%dmV)\n",
+		   charger->adc.vchmax_res,
+		   da9030_reg_to_mA(charger->adc.vchmax_res));
+	seq_printf(s, "vchmin_res = %d (%dmV)\n",
+		   charger->adc.vchmin_res,
+		   da9030_reg_to_mV(charger->adc.vchmin_res));
+
+	return 0;
+}
+
+static int debug_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, bat_debug_show, inode->i_private);
+}
+
+static const struct file_operations bat_debug_fops = {
+	.open		= debug_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)

drivers/power/da9030_battery.c line 195

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+{
+	charger->debug_file = debugfs_create_file("charger", 0666, 0, charger,
+						 &bat_debug_fops);
+	return charger->debug_file;
+}
+
+static void da9030_bat_remove_debugfs(struct da9030_charger *charger)
+{
+	debugfs_remove(charger->debug_file);
+}

I'd put an empty line here, or remove empty line at (1). Just for consistency.

Quoted 25 lines
drivers/power/da9030_battery.c new file +25 -0
+#else
+static inline struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)
+{
+	return NULL;
+}
+static inline void da9030_bat_remove_debugfs(struct da9030_charger *charger)
+{
+}
+#endif
+
+static inline void da9030_read_adc(struct da9030_charger *charger,
+				   struct da9030_adc_res *adc)
+{
+	da903x_reads(charger->master, DA9030_VBAT_RES,
+		     sizeof(*adc), (uint8_t *)adc);
+}
+
+static void da9030_charger_update_state(struct da9030_charger *charger)
+{
+	uint8_t val;
+
+	da903x_read(charger->master, DA9030_CHARGE_CONTROL, &val);
+	charger->is_on = (val & DA9030_CHRG_CHARGER_ENABLE) ? 1 : 0;
+	charger->mA = ((val >> 3) & 0xf) * 100;
+	charger->mV = (val & 0x7) * 50 + 4000;

drivers/power/da9030_battery.c line 230

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+
+	da9030_read_adc(charger, &charger->adc);
+	da903x_read(charger->master, DA9030_FAULT_LOG, &charger->fault);
+	charger->chdet = da903x_query_status(charger->master,
+						     DA9030_STATUS_CHDET);
+}
+
+static void da9030_set_charge(struct da9030_charger *charger, int on)
+{
+	uint8_t val = 0;

= 0 isn't necessary here.

Quoted 132 lines
drivers/power/da9030_battery.c new file +132 -0
+
+	if (on) {
+		val = DA9030_CHRG_CHARGER_ENABLE;
+		val |= (charger->charge_milliamp / 100) << 3;
+		val |= (charger->charge_millivolt - 4000) / 50;
+		charger->is_on = 1;
+	} else {
+		val = 0;
+		charger->is_on = 0;
+	}
+
+	da903x_write(charger->master, DA9030_CHARGE_CONTROL, val);
+}
+
+static void da9030_charger_check_state(struct da9030_charger *charger)
+{
+	da9030_charger_update_state(charger);
+
+	/* we wake or boot with external power on */
+	if (!charger->is_on) {
+		if ((charger->chdet) &&
+		    (charger->adc.vbat_res <
+		     charger->thresholds.vbat_charge_start)) {
+			da9030_set_charge(charger, 1);
+		}
+	} else {
+		if (charger->adc.vbat_res >=
+		    charger->thresholds.vbat_charge_stop) {
+			da9030_set_charge(charger, 0);
+			da903x_write(charger->master, DA9030_VBATMON,
+				       charger->thresholds.vbat_charge_restart);
+		} else if (charger->adc.vbat_res >
+			   charger->thresholds.vbat_low) {
+			/* we are charging and passed LOW_THRESH,
+			   so upate DA9030 VBAT threshold
+			 */
+			da903x_write(charger->master, DA9030_VBATMON,
+				     charger->thresholds.vbat_low);
+		}
+		if (charger->adc.vchmax_res > charger->thresholds.vcharge_max ||
+		    charger->adc.vchmin_res < charger->thresholds.vcharge_min ||
+		    /* Tempreture readings are negative */
+		    charger->adc.tbat_res < charger->thresholds.tbat_high ||
+		    charger->adc.tbat_res > charger->thresholds.tbat_low) {
+			/* disable charger */
+			da9030_set_charge(charger, 0);
+		}
+	}
+}
+
+static void da9030_charging_monitor(struct work_struct *work)
+{
+	struct da9030_charger *charger;
+
+	charger = container_of(work, struct da9030_charger, work.work);
+
+	da9030_charger_check_state(charger);
+
+	/* reschedule for the next time */
+	schedule_delayed_work(&charger->work, charger->interval);
+}
+
+static enum power_supply_property da9030_battery_props[] = {
+	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_AVG,
+};
+
+static void da9030_battery_check_status(struct da9030_charger *charger,
+				    union power_supply_propval *val)
+{
+	if (charger->chdet) {
+		if (charger->is_on)
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+		else
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+	} else {
+		val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+	}
+}
+
+static void da9030_battery_check_health(struct da9030_charger *charger,
+				    union power_supply_propval *val)
+{
+	if (charger->fault & DA9030_FAULT_LOG_OVER_TEMP)
+		val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+	else if (charger->fault & DA9030_FAULT_LOG_VBAT_OVER)
+		val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+	else
+		val->intval = POWER_SUPPLY_HEALTH_GOOD;
+}
+
+static int da9030_battery_get_property(struct power_supply *psy,
+				   enum power_supply_property psp,
+				   union power_supply_propval *val)
+{
+	struct da9030_charger *charger;
+	charger = container_of(psy, struct da9030_charger, psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		da9030_battery_check_status(charger, val);
+		break;
+	case POWER_SUPPLY_PROP_HEALTH:
+		da9030_battery_check_health(charger, val);
+		break;
+	case POWER_SUPPLY_PROP_TECHNOLOGY:
+		val->intval = charger->battery_info->technology;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		val->intval = charger->battery_info->voltage_max_design;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		val->intval = charger->battery_info->voltage_min_design;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = da9030_reg_to_mV(charger->adc.vbat_res) * 1000;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_AVG:
+		val->intval =
+			da9030_reg_to_mA(charger->adc.ichaverage_res) * 1000;
+		break;
+	case POWER_SUPPLY_PROP_MODEL_NAME:
+		val->strval = charger->battery_info->name;
+		break;
+	default:
+		break;

drivers/power/da9030_battery.c line 372

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	}
+
+	return 0;
+}
+
+static void da9030_battery_vbat_event(struct da9030_charger *charger)
+{
+	da9030_read_adc(charger, &charger->adc);
+
+	if (!charger->is_on) {

I'd write it as:

if (charger->is_on)

	return;

the-rest;

Saves one indent level.

Quoted 66 lines
drivers/power/da9030_battery.c new file +66 -0
+		if (charger->adc.vbat_res < charger->thresholds.vbat_low) {
+			/* set VBAT threshold for critical */
+			da903x_write(charger->master, DA9030_VBATMON,
+				     charger->thresholds.vbat_crit);
+			if (charger->battery_low)
+				charger->battery_low();
+		} else if (charger->adc.vbat_res <
+			   charger->thresholds.vbat_crit) {
+			/* notify the system of battery critical */
+			if (charger->battery_critical)
+				charger->battery_critical();
+		}
+	}
+}
+
+static int da9030_battery_event(struct notifier_block *nb, unsigned long event,
+				void *data)
+{
+	struct da9030_charger *charger =
+		container_of(nb, struct da9030_charger, nb);
+	int status;
+
+	switch (event) {
+	case DA9030_EVENT_CHDET:
+		status = da903x_query_status(charger->master,
+					     DA9030_STATUS_CHDET);
+		da9030_set_charge(charger, status);
+		break;
+	case DA9030_EVENT_VBATMON:
+		da9030_battery_vbat_event(charger);
+		break;
+	case DA9030_EVENT_CHIOVER:
+	case DA9030_EVENT_TBAT:
+		da9030_set_charge(charger, 0);
+		break;
+	}
+
+	return 0;
+}
+
+static void da9030_battery_convert_thresholds(struct da9030_charger *charger,
+					      struct da9030_battery_info *pdata)
+{
+	charger->thresholds.tbat_low = pdata->tbat_low;
+	charger->thresholds.tbat_high = pdata->tbat_high;
+	charger->thresholds.tbat_restart  = pdata->tbat_restart;
+
+	charger->thresholds.vbat_low =
+		da9030_millivolt_to_reg(pdata->vbat_low);
+	charger->thresholds.vbat_crit =
+		da9030_millivolt_to_reg(pdata->vbat_crit);
+	charger->thresholds.vbat_charge_start =
+		da9030_millivolt_to_reg(pdata->vbat_charge_start);
+	charger->thresholds.vbat_charge_stop =
+		da9030_millivolt_to_reg(pdata->vbat_charge_stop);
+	charger->thresholds.vbat_charge_restart =
+		da9030_millivolt_to_reg(pdata->vbat_charge_restart);
+
+	charger->thresholds.vcharge_min =
+		da9030_millivolt_to_reg(pdata->vcharge_min);
+	charger->thresholds.vcharge_max =
+		da9030_millivolt_to_reg(pdata->vcharge_max);
+}
+
+static void da9030_battery_setup_psy(struct da9030_charger *charger)
+{

drivers/power/da9030_battery.c line 448

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	struct power_supply *psy = &charger->psy;
+	struct power_supply_info *info = charger->battery_info;
+
+	psy->name = info->name;
+	psy->use_for_apm = info->use_for_apm;
+	psy->type = POWER_SUPPLY_TYPE_BATTERY;
+	psy->get_property = da9030_battery_get_property;
+
+	psy->properties = da9030_battery_props;
+	psy->num_properties = ARRAY_SIZE(da9030_battery_props);;

One semicolon is enough.

Quoted 8 lines
drivers/power/da9030_battery.c new file +8 -0
+};
+
+static int da9030_battery_charger_init(struct da9030_charger *charger)
+{
+	char v[5];
+	int ret;
+
+	v[0] = v[1] = charger->thresholds.vbat_low;

drivers/power/da9030_battery.c line 466

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	v[2] = charger->thresholds.tbat_high;
+	v[3] = charger->thresholds.tbat_restart;
+	v[4] = charger->thresholds.tbat_low;
+
+	ret = da903x_writes(charger->master, DA9030_VBATMON, 5, v);
+	if (ret)
+		return ret;
+
+	/* enable reference voltage supply for ADC from the LDO_INTERNAL
+	   regulator. Must be set before ADC measurements can be made. */

I would suggest using canonical-style comments:

/*
* Not a big deal though, I'm
* just nitpicking. ;-)
*/

Thanks,

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
Signature · 2 lines
Intel Open Source Technology Centre
http://oss.intel.com/
#6 lore raw

Samuel,

Quoted 5 lines · Samuel Ortiz
Quoted 4 lines · Anton Vorontsov

Hi all,

On Thu, Dec 18, 2008 at 01:36:28PM +0200, Mike Rapoport wrote:
[...]

Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines

2 remarks though:
1) I'd like to get Anton Ack on it.

Samuel, as it depends on the MFD part, feel free to merge it into your tree.

Thanks Anton, I just applied Mike's latest version of the patch.

I've fixed the points Anton has raised. Would you prefer me to resend the entire patch or send you incremental patch with fixes?

Quoted 529 lines

Cheers,
Samuel.

Quoted 524 lines
Quoted 8 lines
Quoted 3 lines

2) This patch wont build, as the Makefile is trying to build da903x_battery.o while da9030_battery.c is created. I guess it'd make sense to rename it da903x_battery.c.

Last minute changes...
I actually prefer to keep it da9030 for now because I have no idea how much DA9034 charger differs from DA9030.
So, here's the updated patch that fixes Kconfig and Makefile entries and addresses the points Andrew raised at [1]

Mike,

Thanks for the patch, looks really good. Few comments down below.

[...]

drivers/power/da9030_battery.c line 11

Quoted 13 lines

+++ b/drivers/power/da9030_battery.c

drivers/power/da9030_battery.c new file +11 -0
@@ -0,0 +1,593 @@
+/*
+ * Battery charger driver for Dialog Semiconductor DA9030
+ *
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <[email protected]>
+ *
+ * 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.
+ */
+

The code should explicitly include all the needed headers.

#include <linux/kernel.h> (for container_of, sprintf, ...) #include <linux/init.h> (initcalls)
#include <linux/types.h> (uint8_t)
#include <linux/device.h> (struct device) #include <linux/workqueue.h> (delayedwork)

^^^ This is the only real comment. All the rest are nitpicks, which you can freely ignore if you like.

Quoted 82 lines
drivers/power/da9030_battery.c new file +82 -0
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/mfd/da903x.h>
+
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+
+#define DA9030_STATUS_CHDET	(1 << 3)
+
+#define DA9030_FAULT_LOG		0x0a
+#define DA9030_FAULT_LOG_OVER_TEMP	(1 << 7)
+#define DA9030_FAULT_LOG_VBAT_OVER	(1 << 4)
+
+#define DA9030_CHARGE_CONTROL		0x28
+#define DA9030_CHRG_CHARGER_ENABLE	(1 << 7)
+
+#define DA9030_ADC_MAN_CONTROL		0x30
+#define DA9030_ADC_TBATREF_ENABLE	(1 << 5)
+#define DA9030_ADC_LDO_INT_ENABLE	(1 << 4)
+
+#define DA9030_ADC_AUTO_CONTROL		0x31
+#define DA9030_ADC_TBAT_ENABLE		(1 << 5)
+#define DA9030_ADC_VBAT_IN_TXON		(1 << 4)
+#define DA9030_ADC_VCH_ENABLE		(1 << 3)
+#define DA9030_ADC_ICH_ENABLE		(1 << 2)
+#define DA9030_ADC_VBAT_ENABLE		(1 << 1)
+#define DA9030_ADC_AUTO_SLEEP_ENABLE	(1 << 0)
+
+#define DA9030_VBATMON		0x32
+#define DA9030_VBATMONTXON	0x33
+#define DA9030_TBATHIGHP	0x34
+#define DA9030_TBATHIGHN	0x35
+#define DA9030_TBATLOW		0x36
+
+#define DA9030_VBAT_RES		0x41
+#define DA9030_VBATMIN_RES	0x42
+#define DA9030_VBATMINTXON_RES	0x43
+#define DA9030_ICHMAX_RES	0x44
+#define DA9030_ICHMIN_RES	0x45
+#define DA9030_ICHAVERAGE_RES	0x46
+#define DA9030_VCHMAX_RES	0x47
+#define DA9030_VCHMIN_RES	0x48
+#define DA9030_TBAT_RES		0x49
+
+struct da9030_adc_res {
+	uint8_t vbat_res;
+	uint8_t vbatmin_res;
+	uint8_t vbatmintxon;
+	uint8_t ichmax_res;
+	uint8_t ichmin_res;
+	uint8_t ichaverage_res;
+	uint8_t vchmax_res;
+	uint8_t vchmin_res;
+	uint8_t tbat_res;
+	uint8_t adc_in4_res;
+	uint8_t adc_in5_res;
+};
+
+struct da9030_battery_thresholds {
+	int tbat_low;
+	int tbat_high;
+	int tbat_restart;
+
+	int vbat_low;
+	int vbat_crit;
+	int vbat_charge_start;
+	int vbat_charge_stop;
+	int vbat_charge_restart;
+
+	int vcharge_min;
+	int vcharge_max;
+};
+
+struct da9030_charger {
+	struct power_supply psy;
+
+	struct device *master;
+
+	struct da9030_adc_res adc;
+	struct delayed_work work;
+	unsigned int interval;

drivers/power/da9030_battery.c line 103

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+
+	struct power_supply_info *battery_info;
+
+	struct da9030_battery_thresholds thresholds;
+
+	unsigned int charge_milliamp;
+	unsigned int charge_millivolt;
+
+	/* charger status */
+	int chdet:1;

In kernel we can use bool type, and true/false constants.

Quoted 21 lines
drivers/power/da9030_battery.c new file +21 -0
+	uint8_t fault;
+	int mA;
+	int mV;
+	int is_on;
+
+	struct notifier_block nb;
+
+	/* platform callbacks for battery low and critical events */
+	void (*battery_low)(void);
+	void (*battery_critical)(void);
+
+	struct dentry *debug_file;
+};
+
+static inline int da9030_reg_to_mV(int reg)
+{
+	return ((reg * 2650) >> 8) + 2650;
+}
+
+static inline int da9030_millivolt_to_reg(int mV)
+{

drivers/power/da9030_battery.c line 134

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	return ((mV - 2650) << 8) / 2650;
+}
+
+static inline int da9030_reg_to_mA(int reg)
+{
+	return ((reg * 24000) >> 8) / 15;
+}
+
+#ifdef CONFIG_DEBUG_FS
+

(1)

Quoted 51 lines
drivers/power/da9030_battery.c new file +51 -0
+static int bat_debug_show(struct seq_file *s, void *data)
+{
+	struct da9030_charger *charger = s->private;
+
+	seq_printf(s, "charger is %s\n", charger->is_on ? "on" : "off");
+	if (charger->chdet) {
+		seq_printf(s, "iset = %dmA, vset = %dmV\n",
+			   charger->mA, charger->mV);
+	}
+
+	seq_printf(s, "vbat_res = %d (%dmV)\n",
+		   charger->adc.vbat_res,
+		   da9030_reg_to_mV(charger->adc.vbat_res));
+	seq_printf(s, "vbatmin_res = %d (%dmV)\n",
+		   charger->adc.vbatmin_res,
+		   da9030_reg_to_mV(charger->adc.vbatmin_res));
+	seq_printf(s, "vbatmintxon = %d (%dmV)\n",
+		   charger->adc.vbatmintxon,
+		   da9030_reg_to_mV(charger->adc.vbatmintxon));
+	seq_printf(s, "ichmax_res = %d (%dmA)\n",
+		   charger->adc.ichmax_res,
+		   da9030_reg_to_mV(charger->adc.ichmax_res));
+	seq_printf(s, "ichmin_res = %d (%dmA)\n",
+		   charger->adc.ichmin_res,
+		   da9030_reg_to_mA(charger->adc.ichmin_res));
+	seq_printf(s, "ichaverage_res = %d (%dmA)\n",
+		   charger->adc.ichaverage_res,
+		   da9030_reg_to_mA(charger->adc.ichaverage_res));
+	seq_printf(s, "vchmax_res = %d (%dmV)\n",
+		   charger->adc.vchmax_res,
+		   da9030_reg_to_mA(charger->adc.vchmax_res));
+	seq_printf(s, "vchmin_res = %d (%dmV)\n",
+		   charger->adc.vchmin_res,
+		   da9030_reg_to_mV(charger->adc.vchmin_res));
+
+	return 0;
+}
+
+static int debug_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, bat_debug_show, inode->i_private);
+}
+
+static const struct file_operations bat_debug_fops = {
+	.open		= debug_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)

drivers/power/da9030_battery.c line 195

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+{
+	charger->debug_file = debugfs_create_file("charger", 0666, 0, charger,
+						 &bat_debug_fops);
+	return charger->debug_file;
+}
+
+static void da9030_bat_remove_debugfs(struct da9030_charger *charger)
+{
+	debugfs_remove(charger->debug_file);
+}

I'd put an empty line here, or remove empty line at (1). Just for consistency.

Quoted 25 lines
drivers/power/da9030_battery.c new file +25 -0
+#else
+static inline struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)
+{
+	return NULL;
+}
+static inline void da9030_bat_remove_debugfs(struct da9030_charger *charger)
+{
+}
+#endif
+
+static inline void da9030_read_adc(struct da9030_charger *charger,
+				   struct da9030_adc_res *adc)
+{
+	da903x_reads(charger->master, DA9030_VBAT_RES,
+		     sizeof(*adc), (uint8_t *)adc);
+}
+
+static void da9030_charger_update_state(struct da9030_charger *charger)
+{
+	uint8_t val;
+
+	da903x_read(charger->master, DA9030_CHARGE_CONTROL, &val);
+	charger->is_on = (val & DA9030_CHRG_CHARGER_ENABLE) ? 1 : 0;
+	charger->mA = ((val >> 3) & 0xf) * 100;
+	charger->mV = (val & 0x7) * 50 + 4000;

drivers/power/da9030_battery.c line 230

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+
+	da9030_read_adc(charger, &charger->adc);
+	da903x_read(charger->master, DA9030_FAULT_LOG, &charger->fault);
+	charger->chdet = da903x_query_status(charger->master,
+						     DA9030_STATUS_CHDET);
+}
+
+static void da9030_set_charge(struct da9030_charger *charger, int on)
+{
+	uint8_t val = 0;

= 0 isn't necessary here.

Quoted 132 lines
drivers/power/da9030_battery.c new file +132 -0
+
+	if (on) {
+		val = DA9030_CHRG_CHARGER_ENABLE;
+		val |= (charger->charge_milliamp / 100) << 3;
+		val |= (charger->charge_millivolt - 4000) / 50;
+		charger->is_on = 1;
+	} else {
+		val = 0;
+		charger->is_on = 0;
+	}
+
+	da903x_write(charger->master, DA9030_CHARGE_CONTROL, val);
+}
+
+static void da9030_charger_check_state(struct da9030_charger *charger)
+{
+	da9030_charger_update_state(charger);
+
+	/* we wake or boot with external power on */
+	if (!charger->is_on) {
+		if ((charger->chdet) &&
+		    (charger->adc.vbat_res <
+		     charger->thresholds.vbat_charge_start)) {
+			da9030_set_charge(charger, 1);
+		}
+	} else {
+		if (charger->adc.vbat_res >=
+		    charger->thresholds.vbat_charge_stop) {
+			da9030_set_charge(charger, 0);
+			da903x_write(charger->master, DA9030_VBATMON,
+				       charger->thresholds.vbat_charge_restart);
+		} else if (charger->adc.vbat_res >
+			   charger->thresholds.vbat_low) {
+			/* we are charging and passed LOW_THRESH,
+			   so upate DA9030 VBAT threshold
+			 */
+			da903x_write(charger->master, DA9030_VBATMON,
+				     charger->thresholds.vbat_low);
+		}
+		if (charger->adc.vchmax_res > charger->thresholds.vcharge_max ||
+		    charger->adc.vchmin_res < charger->thresholds.vcharge_min ||
+		    /* Tempreture readings are negative */
+		    charger->adc.tbat_res < charger->thresholds.tbat_high ||
+		    charger->adc.tbat_res > charger->thresholds.tbat_low) {
+			/* disable charger */
+			da9030_set_charge(charger, 0);
+		}
+	}
+}
+
+static void da9030_charging_monitor(struct work_struct *work)
+{
+	struct da9030_charger *charger;
+
+	charger = container_of(work, struct da9030_charger, work.work);
+
+	da9030_charger_check_state(charger);
+
+	/* reschedule for the next time */
+	schedule_delayed_work(&charger->work, charger->interval);
+}
+
+static enum power_supply_property da9030_battery_props[] = {
+	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_AVG,
+};
+
+static void da9030_battery_check_status(struct da9030_charger *charger,
+				    union power_supply_propval *val)
+{
+	if (charger->chdet) {
+		if (charger->is_on)
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+		else
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+	} else {
+		val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+	}
+}
+
+static void da9030_battery_check_health(struct da9030_charger *charger,
+				    union power_supply_propval *val)
+{
+	if (charger->fault & DA9030_FAULT_LOG_OVER_TEMP)
+		val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+	else if (charger->fault & DA9030_FAULT_LOG_VBAT_OVER)
+		val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+	else
+		val->intval = POWER_SUPPLY_HEALTH_GOOD;
+}
+
+static int da9030_battery_get_property(struct power_supply *psy,
+				   enum power_supply_property psp,
+				   union power_supply_propval *val)
+{
+	struct da9030_charger *charger;
+	charger = container_of(psy, struct da9030_charger, psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		da9030_battery_check_status(charger, val);
+		break;
+	case POWER_SUPPLY_PROP_HEALTH:
+		da9030_battery_check_health(charger, val);
+		break;
+	case POWER_SUPPLY_PROP_TECHNOLOGY:
+		val->intval = charger->battery_info->technology;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		val->intval = charger->battery_info->voltage_max_design;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		val->intval = charger->battery_info->voltage_min_design;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = da9030_reg_to_mV(charger->adc.vbat_res) * 1000;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_AVG:
+		val->intval =
+			da9030_reg_to_mA(charger->adc.ichaverage_res) * 1000;
+		break;
+	case POWER_SUPPLY_PROP_MODEL_NAME:
+		val->strval = charger->battery_info->name;
+		break;
+	default:
+		break;

drivers/power/da9030_battery.c line 372

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	}
+
+	return 0;
+}
+
+static void da9030_battery_vbat_event(struct da9030_charger *charger)
+{
+	da9030_read_adc(charger, &charger->adc);
+
+	if (!charger->is_on) {

I'd write it as:

if (charger->is_on)

	return;

the-rest;

Saves one indent level.

Quoted 66 lines
drivers/power/da9030_battery.c new file +66 -0
+		if (charger->adc.vbat_res < charger->thresholds.vbat_low) {
+			/* set VBAT threshold for critical */
+			da903x_write(charger->master, DA9030_VBATMON,
+				     charger->thresholds.vbat_crit);
+			if (charger->battery_low)
+				charger->battery_low();
+		} else if (charger->adc.vbat_res <
+			   charger->thresholds.vbat_crit) {
+			/* notify the system of battery critical */
+			if (charger->battery_critical)
+				charger->battery_critical();
+		}
+	}
+}
+
+static int da9030_battery_event(struct notifier_block *nb, unsigned long event,
+				void *data)
+{
+	struct da9030_charger *charger =
+		container_of(nb, struct da9030_charger, nb);
+	int status;
+
+	switch (event) {
+	case DA9030_EVENT_CHDET:
+		status = da903x_query_status(charger->master,
+					     DA9030_STATUS_CHDET);
+		da9030_set_charge(charger, status);
+		break;
+	case DA9030_EVENT_VBATMON:
+		da9030_battery_vbat_event(charger);
+		break;
+	case DA9030_EVENT_CHIOVER:
+	case DA9030_EVENT_TBAT:
+		da9030_set_charge(charger, 0);
+		break;
+	}
+
+	return 0;
+}
+
+static void da9030_battery_convert_thresholds(struct da9030_charger *charger,
+					      struct da9030_battery_info *pdata)
+{
+	charger->thresholds.tbat_low = pdata->tbat_low;
+	charger->thresholds.tbat_high = pdata->tbat_high;
+	charger->thresholds.tbat_restart  = pdata->tbat_restart;
+
+	charger->thresholds.vbat_low =
+		da9030_millivolt_to_reg(pdata->vbat_low);
+	charger->thresholds.vbat_crit =
+		da9030_millivolt_to_reg(pdata->vbat_crit);
+	charger->thresholds.vbat_charge_start =
+		da9030_millivolt_to_reg(pdata->vbat_charge_start);
+	charger->thresholds.vbat_charge_stop =
+		da9030_millivolt_to_reg(pdata->vbat_charge_stop);
+	charger->thresholds.vbat_charge_restart =
+		da9030_millivolt_to_reg(pdata->vbat_charge_restart);
+
+	charger->thresholds.vcharge_min =
+		da9030_millivolt_to_reg(pdata->vcharge_min);
+	charger->thresholds.vcharge_max =
+		da9030_millivolt_to_reg(pdata->vcharge_max);
+}
+
+static void da9030_battery_setup_psy(struct da9030_charger *charger)
+{

drivers/power/da9030_battery.c line 448

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	struct power_supply *psy = &charger->psy;
+	struct power_supply_info *info = charger->battery_info;
+
+	psy->name = info->name;
+	psy->use_for_apm = info->use_for_apm;
+	psy->type = POWER_SUPPLY_TYPE_BATTERY;
+	psy->get_property = da9030_battery_get_property;
+
+	psy->properties = da9030_battery_props;
+	psy->num_properties = ARRAY_SIZE(da9030_battery_props);;

One semicolon is enough.

Quoted 8 lines
drivers/power/da9030_battery.c new file +8 -0
+};
+
+static int da9030_battery_charger_init(struct da9030_charger *charger)
+{
+	char v[5];
+	int ret;
+
+	v[0] = v[1] = charger->thresholds.vbat_low;

drivers/power/da9030_battery.c line 466

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	v[2] = charger->thresholds.tbat_high;
+	v[3] = charger->thresholds.tbat_restart;
+	v[4] = charger->thresholds.tbat_low;
+
+	ret = da903x_writes(charger->master, DA9030_VBATMON, 5, v);
+	if (ret)
+		return ret;
+
+	/* enable reference voltage supply for ADC from the LDO_INTERNAL
+	   regulator. Must be set before ADC measurements can be made. */

I would suggest using canonical-style comments:

/*
* Not a big deal though, I'm
* just nitpicking. ;-)
*/

Thanks,

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
Signature · 2 lines
Sincerely yours,
Mike.
#7 lore raw
Quoted 10 lines · Mike Rapoport

Samuel,

Quoted 7 lines · Samuel Ortiz
Quoted 6 lines · Anton Vorontsov

Hi all,

On Thu, Dec 18, 2008 at 01:36:28PM +0200, Mike Rapoport wrote:
[...]

Quoted 2 lines
Quoted 2 lines

2 remarks though:
1) I'd like to get Anton Ack on it.

Quoted 8 lines
Quoted 5 lines
Quoted 4 lines

Samuel, as it depends on the MFD part, feel free to merge it into your tree.

Thanks Anton, I just applied Mike's latest version of the patch.

I've fixed the points Anton has raised. Would you prefer me to resend the entire patch or send you incremental patch with fixes?

I'd prefer to get the whole patch, thanks.

Cheers,
Samuel.

Quoted 534 lines
Quoted 529 lines

Cheers,
Samuel.

Quoted 524 lines
Quoted 8 lines
Quoted 3 lines

2) This patch wont build, as the Makefile is trying to build da903x_battery.o while da9030_battery.c is created. I guess it'd make sense to rename it da903x_battery.c.

Last minute changes...
I actually prefer to keep it da9030 for now because I have no idea how much DA9034 charger differs from DA9030.
So, here's the updated patch that fixes Kconfig and Makefile entries and addresses the points Andrew raised at [1]

Mike,

Thanks for the patch, looks really good. Few comments down below.

[...]

drivers/power/da9030_battery.c line 11

Quoted 13 lines

+++ b/drivers/power/da9030_battery.c

drivers/power/da9030_battery.c new file +11 -0
@@ -0,0 +1,593 @@
+/*
+ * Battery charger driver for Dialog Semiconductor DA9030
+ *
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <[email protected]>
+ *
+ * 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.
+ */
+

The code should explicitly include all the needed headers.

#include <linux/kernel.h> (for container_of, sprintf, ...) #include <linux/init.h> (initcalls)
#include <linux/types.h> (uint8_t)
#include <linux/device.h> (struct device) #include <linux/workqueue.h> (delayedwork)

^^^ This is the only real comment. All the rest are nitpicks, which you can freely ignore if you like.

Quoted 82 lines
drivers/power/da9030_battery.c new file +82 -0
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/mfd/da903x.h>
+
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+
+#define DA9030_STATUS_CHDET	(1 << 3)
+
+#define DA9030_FAULT_LOG		0x0a
+#define DA9030_FAULT_LOG_OVER_TEMP	(1 << 7)
+#define DA9030_FAULT_LOG_VBAT_OVER	(1 << 4)
+
+#define DA9030_CHARGE_CONTROL		0x28
+#define DA9030_CHRG_CHARGER_ENABLE	(1 << 7)
+
+#define DA9030_ADC_MAN_CONTROL		0x30
+#define DA9030_ADC_TBATREF_ENABLE	(1 << 5)
+#define DA9030_ADC_LDO_INT_ENABLE	(1 << 4)
+
+#define DA9030_ADC_AUTO_CONTROL		0x31
+#define DA9030_ADC_TBAT_ENABLE		(1 << 5)
+#define DA9030_ADC_VBAT_IN_TXON		(1 << 4)
+#define DA9030_ADC_VCH_ENABLE		(1 << 3)
+#define DA9030_ADC_ICH_ENABLE		(1 << 2)
+#define DA9030_ADC_VBAT_ENABLE		(1 << 1)
+#define DA9030_ADC_AUTO_SLEEP_ENABLE	(1 << 0)
+
+#define DA9030_VBATMON		0x32
+#define DA9030_VBATMONTXON	0x33
+#define DA9030_TBATHIGHP	0x34
+#define DA9030_TBATHIGHN	0x35
+#define DA9030_TBATLOW		0x36
+
+#define DA9030_VBAT_RES		0x41
+#define DA9030_VBATMIN_RES	0x42
+#define DA9030_VBATMINTXON_RES	0x43
+#define DA9030_ICHMAX_RES	0x44
+#define DA9030_ICHMIN_RES	0x45
+#define DA9030_ICHAVERAGE_RES	0x46
+#define DA9030_VCHMAX_RES	0x47
+#define DA9030_VCHMIN_RES	0x48
+#define DA9030_TBAT_RES		0x49
+
+struct da9030_adc_res {
+	uint8_t vbat_res;
+	uint8_t vbatmin_res;
+	uint8_t vbatmintxon;
+	uint8_t ichmax_res;
+	uint8_t ichmin_res;
+	uint8_t ichaverage_res;
+	uint8_t vchmax_res;
+	uint8_t vchmin_res;
+	uint8_t tbat_res;
+	uint8_t adc_in4_res;
+	uint8_t adc_in5_res;
+};
+
+struct da9030_battery_thresholds {
+	int tbat_low;
+	int tbat_high;
+	int tbat_restart;
+
+	int vbat_low;
+	int vbat_crit;
+	int vbat_charge_start;
+	int vbat_charge_stop;
+	int vbat_charge_restart;
+
+	int vcharge_min;
+	int vcharge_max;
+};
+
+struct da9030_charger {
+	struct power_supply psy;
+
+	struct device *master;
+
+	struct da9030_adc_res adc;
+	struct delayed_work work;
+	unsigned int interval;

drivers/power/da9030_battery.c line 103

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+
+	struct power_supply_info *battery_info;
+
+	struct da9030_battery_thresholds thresholds;
+
+	unsigned int charge_milliamp;
+	unsigned int charge_millivolt;
+
+	/* charger status */
+	int chdet:1;

In kernel we can use bool type, and true/false constants.

Quoted 21 lines
drivers/power/da9030_battery.c new file +21 -0
+	uint8_t fault;
+	int mA;
+	int mV;
+	int is_on;
+
+	struct notifier_block nb;
+
+	/* platform callbacks for battery low and critical events */
+	void (*battery_low)(void);
+	void (*battery_critical)(void);
+
+	struct dentry *debug_file;
+};
+
+static inline int da9030_reg_to_mV(int reg)
+{
+	return ((reg * 2650) >> 8) + 2650;
+}
+
+static inline int da9030_millivolt_to_reg(int mV)
+{

drivers/power/da9030_battery.c line 134

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	return ((mV - 2650) << 8) / 2650;
+}
+
+static inline int da9030_reg_to_mA(int reg)
+{
+	return ((reg * 24000) >> 8) / 15;
+}
+
+#ifdef CONFIG_DEBUG_FS
+

(1)

Quoted 51 lines
drivers/power/da9030_battery.c new file +51 -0
+static int bat_debug_show(struct seq_file *s, void *data)
+{
+	struct da9030_charger *charger = s->private;
+
+	seq_printf(s, "charger is %s\n", charger->is_on ? "on" : "off");
+	if (charger->chdet) {
+		seq_printf(s, "iset = %dmA, vset = %dmV\n",
+			   charger->mA, charger->mV);
+	}
+
+	seq_printf(s, "vbat_res = %d (%dmV)\n",
+		   charger->adc.vbat_res,
+		   da9030_reg_to_mV(charger->adc.vbat_res));
+	seq_printf(s, "vbatmin_res = %d (%dmV)\n",
+		   charger->adc.vbatmin_res,
+		   da9030_reg_to_mV(charger->adc.vbatmin_res));
+	seq_printf(s, "vbatmintxon = %d (%dmV)\n",
+		   charger->adc.vbatmintxon,
+		   da9030_reg_to_mV(charger->adc.vbatmintxon));
+	seq_printf(s, "ichmax_res = %d (%dmA)\n",
+		   charger->adc.ichmax_res,
+		   da9030_reg_to_mV(charger->adc.ichmax_res));
+	seq_printf(s, "ichmin_res = %d (%dmA)\n",
+		   charger->adc.ichmin_res,
+		   da9030_reg_to_mA(charger->adc.ichmin_res));
+	seq_printf(s, "ichaverage_res = %d (%dmA)\n",
+		   charger->adc.ichaverage_res,
+		   da9030_reg_to_mA(charger->adc.ichaverage_res));
+	seq_printf(s, "vchmax_res = %d (%dmV)\n",
+		   charger->adc.vchmax_res,
+		   da9030_reg_to_mA(charger->adc.vchmax_res));
+	seq_printf(s, "vchmin_res = %d (%dmV)\n",
+		   charger->adc.vchmin_res,
+		   da9030_reg_to_mV(charger->adc.vchmin_res));
+
+	return 0;
+}
+
+static int debug_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, bat_debug_show, inode->i_private);
+}
+
+static const struct file_operations bat_debug_fops = {
+	.open		= debug_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)

drivers/power/da9030_battery.c line 195

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+{
+	charger->debug_file = debugfs_create_file("charger", 0666, 0, charger,
+						 &bat_debug_fops);
+	return charger->debug_file;
+}
+
+static void da9030_bat_remove_debugfs(struct da9030_charger *charger)
+{
+	debugfs_remove(charger->debug_file);
+}

I'd put an empty line here, or remove empty line at (1). Just for consistency.

Quoted 25 lines
drivers/power/da9030_battery.c new file +25 -0
+#else
+static inline struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)
+{
+	return NULL;
+}
+static inline void da9030_bat_remove_debugfs(struct da9030_charger *charger)
+{
+}
+#endif
+
+static inline void da9030_read_adc(struct da9030_charger *charger,
+				   struct da9030_adc_res *adc)
+{
+	da903x_reads(charger->master, DA9030_VBAT_RES,
+		     sizeof(*adc), (uint8_t *)adc);
+}
+
+static void da9030_charger_update_state(struct da9030_charger *charger)
+{
+	uint8_t val;
+
+	da903x_read(charger->master, DA9030_CHARGE_CONTROL, &val);
+	charger->is_on = (val & DA9030_CHRG_CHARGER_ENABLE) ? 1 : 0;
+	charger->mA = ((val >> 3) & 0xf) * 100;
+	charger->mV = (val & 0x7) * 50 + 4000;

drivers/power/da9030_battery.c line 230

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+
+	da9030_read_adc(charger, &charger->adc);
+	da903x_read(charger->master, DA9030_FAULT_LOG, &charger->fault);
+	charger->chdet = da903x_query_status(charger->master,
+						     DA9030_STATUS_CHDET);
+}
+
+static void da9030_set_charge(struct da9030_charger *charger, int on)
+{
+	uint8_t val = 0;

= 0 isn't necessary here.

Quoted 132 lines
drivers/power/da9030_battery.c new file +132 -0
+
+	if (on) {
+		val = DA9030_CHRG_CHARGER_ENABLE;
+		val |= (charger->charge_milliamp / 100) << 3;
+		val |= (charger->charge_millivolt - 4000) / 50;
+		charger->is_on = 1;
+	} else {
+		val = 0;
+		charger->is_on = 0;
+	}
+
+	da903x_write(charger->master, DA9030_CHARGE_CONTROL, val);
+}
+
+static void da9030_charger_check_state(struct da9030_charger *charger)
+{
+	da9030_charger_update_state(charger);
+
+	/* we wake or boot with external power on */
+	if (!charger->is_on) {
+		if ((charger->chdet) &&
+		    (charger->adc.vbat_res <
+		     charger->thresholds.vbat_charge_start)) {
+			da9030_set_charge(charger, 1);
+		}
+	} else {
+		if (charger->adc.vbat_res >=
+		    charger->thresholds.vbat_charge_stop) {
+			da9030_set_charge(charger, 0);
+			da903x_write(charger->master, DA9030_VBATMON,
+				       charger->thresholds.vbat_charge_restart);
+		} else if (charger->adc.vbat_res >
+			   charger->thresholds.vbat_low) {
+			/* we are charging and passed LOW_THRESH,
+			   so upate DA9030 VBAT threshold
+			 */
+			da903x_write(charger->master, DA9030_VBATMON,
+				     charger->thresholds.vbat_low);
+		}
+		if (charger->adc.vchmax_res > charger->thresholds.vcharge_max ||
+		    charger->adc.vchmin_res < charger->thresholds.vcharge_min ||
+		    /* Tempreture readings are negative */
+		    charger->adc.tbat_res < charger->thresholds.tbat_high ||
+		    charger->adc.tbat_res > charger->thresholds.tbat_low) {
+			/* disable charger */
+			da9030_set_charge(charger, 0);
+		}
+	}
+}
+
+static void da9030_charging_monitor(struct work_struct *work)
+{
+	struct da9030_charger *charger;
+
+	charger = container_of(work, struct da9030_charger, work.work);
+
+	da9030_charger_check_state(charger);
+
+	/* reschedule for the next time */
+	schedule_delayed_work(&charger->work, charger->interval);
+}
+
+static enum power_supply_property da9030_battery_props[] = {
+	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_AVG,
+};
+
+static void da9030_battery_check_status(struct da9030_charger *charger,
+				    union power_supply_propval *val)
+{
+	if (charger->chdet) {
+		if (charger->is_on)
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+		else
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+	} else {
+		val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+	}
+}
+
+static void da9030_battery_check_health(struct da9030_charger *charger,
+				    union power_supply_propval *val)
+{
+	if (charger->fault & DA9030_FAULT_LOG_OVER_TEMP)
+		val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+	else if (charger->fault & DA9030_FAULT_LOG_VBAT_OVER)
+		val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+	else
+		val->intval = POWER_SUPPLY_HEALTH_GOOD;
+}
+
+static int da9030_battery_get_property(struct power_supply *psy,
+				   enum power_supply_property psp,
+				   union power_supply_propval *val)
+{
+	struct da9030_charger *charger;
+	charger = container_of(psy, struct da9030_charger, psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		da9030_battery_check_status(charger, val);
+		break;
+	case POWER_SUPPLY_PROP_HEALTH:
+		da9030_battery_check_health(charger, val);
+		break;
+	case POWER_SUPPLY_PROP_TECHNOLOGY:
+		val->intval = charger->battery_info->technology;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		val->intval = charger->battery_info->voltage_max_design;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		val->intval = charger->battery_info->voltage_min_design;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = da9030_reg_to_mV(charger->adc.vbat_res) * 1000;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_AVG:
+		val->intval =
+			da9030_reg_to_mA(charger->adc.ichaverage_res) * 1000;
+		break;
+	case POWER_SUPPLY_PROP_MODEL_NAME:
+		val->strval = charger->battery_info->name;
+		break;
+	default:
+		break;

drivers/power/da9030_battery.c line 372

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	}
+
+	return 0;
+}
+
+static void da9030_battery_vbat_event(struct da9030_charger *charger)
+{
+	da9030_read_adc(charger, &charger->adc);
+
+	if (!charger->is_on) {

I'd write it as:

if (charger->is_on)

	return;

the-rest;

Saves one indent level.

Quoted 66 lines
drivers/power/da9030_battery.c new file +66 -0
+		if (charger->adc.vbat_res < charger->thresholds.vbat_low) {
+			/* set VBAT threshold for critical */
+			da903x_write(charger->master, DA9030_VBATMON,
+				     charger->thresholds.vbat_crit);
+			if (charger->battery_low)
+				charger->battery_low();
+		} else if (charger->adc.vbat_res <
+			   charger->thresholds.vbat_crit) {
+			/* notify the system of battery critical */
+			if (charger->battery_critical)
+				charger->battery_critical();
+		}
+	}
+}
+
+static int da9030_battery_event(struct notifier_block *nb, unsigned long event,
+				void *data)
+{
+	struct da9030_charger *charger =
+		container_of(nb, struct da9030_charger, nb);
+	int status;
+
+	switch (event) {
+	case DA9030_EVENT_CHDET:
+		status = da903x_query_status(charger->master,
+					     DA9030_STATUS_CHDET);
+		da9030_set_charge(charger, status);
+		break;
+	case DA9030_EVENT_VBATMON:
+		da9030_battery_vbat_event(charger);
+		break;
+	case DA9030_EVENT_CHIOVER:
+	case DA9030_EVENT_TBAT:
+		da9030_set_charge(charger, 0);
+		break;
+	}
+
+	return 0;
+}
+
+static void da9030_battery_convert_thresholds(struct da9030_charger *charger,
+					      struct da9030_battery_info *pdata)
+{
+	charger->thresholds.tbat_low = pdata->tbat_low;
+	charger->thresholds.tbat_high = pdata->tbat_high;
+	charger->thresholds.tbat_restart  = pdata->tbat_restart;
+
+	charger->thresholds.vbat_low =
+		da9030_millivolt_to_reg(pdata->vbat_low);
+	charger->thresholds.vbat_crit =
+		da9030_millivolt_to_reg(pdata->vbat_crit);
+	charger->thresholds.vbat_charge_start =
+		da9030_millivolt_to_reg(pdata->vbat_charge_start);
+	charger->thresholds.vbat_charge_stop =
+		da9030_millivolt_to_reg(pdata->vbat_charge_stop);
+	charger->thresholds.vbat_charge_restart =
+		da9030_millivolt_to_reg(pdata->vbat_charge_restart);
+
+	charger->thresholds.vcharge_min =
+		da9030_millivolt_to_reg(pdata->vcharge_min);
+	charger->thresholds.vcharge_max =
+		da9030_millivolt_to_reg(pdata->vcharge_max);
+}
+
+static void da9030_battery_setup_psy(struct da9030_charger *charger)
+{

drivers/power/da9030_battery.c line 448

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	struct power_supply *psy = &charger->psy;
+	struct power_supply_info *info = charger->battery_info;
+
+	psy->name = info->name;
+	psy->use_for_apm = info->use_for_apm;
+	psy->type = POWER_SUPPLY_TYPE_BATTERY;
+	psy->get_property = da9030_battery_get_property;
+
+	psy->properties = da9030_battery_props;
+	psy->num_properties = ARRAY_SIZE(da9030_battery_props);;

One semicolon is enough.

Quoted 8 lines
drivers/power/da9030_battery.c new file +8 -0
+};
+
+static int da9030_battery_charger_init(struct da9030_charger *charger)
+{
+	char v[5];
+	int ret;
+
+	v[0] = v[1] = charger->thresholds.vbat_low;

drivers/power/da9030_battery.c line 466

Quoted 10 lines
drivers/power/da9030_battery.c new file +10 -0
+	v[2] = charger->thresholds.tbat_high;
+	v[3] = charger->thresholds.tbat_restart;
+	v[4] = charger->thresholds.tbat_low;
+
+	ret = da903x_writes(charger->master, DA9030_VBATMON, 5, v);
+	if (ret)
+		return ret;
+
+	/* enable reference voltage supply for ADC from the LDO_INTERNAL
+	   regulator. Must be set before ADC measurements can be made. */

I would suggest using canonical-style comments:

/*
* Not a big deal though, I'm
* just nitpicking. ;-)
*/

Thanks,

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
Signature · 2 lines
Sincerely yours,
Mike.
Signature · 2 lines
Intel Open Source Technology Centre
http://oss.intel.com/
author Acked-by Anton Vorontsov
#8 lore raw
Quoted 11 lines · Samuel Ortiz
Quoted 10 lines · Mike Rapoport

Samuel,

Quoted 7 lines · Samuel Ortiz
Quoted 6 lines · Anton Vorontsov

Hi all,

On Thu, Dec 18, 2008 at 01:36:28PM +0200, Mike Rapoport wrote:
[...]

Quoted 2 lines
Quoted 2 lines

2 remarks though:
1) I'd like to get Anton Ack on it.

Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines

Samuel, as it depends on the MFD part, feel free to merge it into your tree.

Thanks Anton, I just applied Mike's latest version of the patch.

I've fixed the points Anton has raised. Would you prefer me to resend the entire patch or send you incremental patch with fixes?

I'd prefer to get the whole patch, thanks.

Ok.

Quoted 2 lines

Cheers,
Samuel.

 drivers/power/Kconfig          |    7 +
 drivers/power/Makefile         |    1 +
 drivers/power/da9030_battery.c |  600 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 608 insertions(+), 0 deletions(-)
drivers/power/Kconfig +7 -0
index 8e0c2b4..afa17b9 100644
@@ -68,4 +68,11 @@ config BATTERY_BQ27x00
 	help
 	  Say Y here to enable support for batteries with BQ27200(I2C) chip.
 
+config BATTERY_DA9030
+	tristate "DA9030 battery driver"
+	depends on PMIC_DA903X
+	help
+	  Say Y here to enable support for batteries charger integrated into
+	  DA9030 PMIC.
+
 endif # POWER_SUPPLY
drivers/power/Makefile +1 -0
index e8f1ece..1c83ede 100644
@@ -23,3 +23,4 @@ obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
 obj-$(CONFIG_BATTERY_TOSA)	+= tosa_battery.o
 obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
 obj-$(CONFIG_BATTERY_BQ27x00)	+= bq27x00_battery.o
+obj-$(CONFIG_BATTERY_DA9030)	+= da9030_battery.o
drivers/power/da9030_battery.c new file +600 -0

600 lines not on this page · raw message

Signature · 2 lines
Sincerely yours,
Mike.
#9 lore raw
Quoted 15 lines · Mike Rapoport
Quoted 14 lines · Samuel Ortiz
Quoted 13 lines · Mike Rapoport

Samuel,

Quoted 10 lines · Samuel Ortiz
Quoted 9 lines · Anton Vorontsov

Hi all,

On Thu, Dec 18, 2008 at 01:36:28PM +0200, Mike Rapoport wrote:
[...]

Quoted 2 lines
Quoted 2 lines

2 remarks though:
1) I'd like to get Anton Ack on it.

Samuel, as it depends on the MFD part, feel free to merge it into your tree.

Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
Quoted 1 line

Thanks Anton, I just applied Mike's latest version of the patch.

I've fixed the points Anton has raised. Would you prefer me to resend the entire patch or send you incremental patch with fixes?

I'd prefer to get the whole patch, thanks.

Ok.

Thanks Mike, applied to my mfd tree.

Cheers,
Samuel.

Quoted 649 lines
Quoted 2 lines

Cheers,
Samuel.

 drivers/power/Kconfig          |    7 +
 drivers/power/Makefile         |    1 +
 drivers/power/da9030_battery.c |  600 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 608 insertions(+), 0 deletions(-)
drivers/power/Kconfig +7 -0
index 8e0c2b4..afa17b9 100644
@@ -68,4 +68,11 @@ config BATTERY_BQ27x00
 	help
 	  Say Y here to enable support for batteries with BQ27200(I2C) chip.
 
+config BATTERY_DA9030
+	tristate "DA9030 battery driver"
+	depends on PMIC_DA903X
+	help
+	  Say Y here to enable support for batteries charger integrated into
+	  DA9030 PMIC.
+
 endif # POWER_SUPPLY
drivers/power/Makefile +1 -0
index e8f1ece..1c83ede 100644
@@ -23,3 +23,4 @@ obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
 obj-$(CONFIG_BATTERY_TOSA)	+= tosa_battery.o
 obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
 obj-$(CONFIG_BATTERY_BQ27x00)	+= bq27x00_battery.o
+obj-$(CONFIG_BATTERY_DA9030)	+= da9030_battery.o
drivers/power/da9030_battery.c new file +600 -0
new file mode 100644
index 0000000..1662bb0
@@ -0,0 +1,600 @@
+/*
+ * Battery charger driver for Dialog Semiconductor DA9030
+ *
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <[email protected]>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/workqueue.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/mfd/da903x.h>
+
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+
+#define DA9030_STATUS_CHDET	(1 << 3)
+
+#define DA9030_FAULT_LOG		0x0a
+#define DA9030_FAULT_LOG_OVER_TEMP	(1 << 7)
+#define DA9030_FAULT_LOG_VBAT_OVER	(1 << 4)
+
+#define DA9030_CHARGE_CONTROL		0x28
+#define DA9030_CHRG_CHARGER_ENABLE	(1 << 7)
+
+#define DA9030_ADC_MAN_CONTROL		0x30
+#define DA9030_ADC_TBATREF_ENABLE	(1 << 5)
+#define DA9030_ADC_LDO_INT_ENABLE	(1 << 4)
+
+#define DA9030_ADC_AUTO_CONTROL		0x31
+#define DA9030_ADC_TBAT_ENABLE		(1 << 5)
+#define DA9030_ADC_VBAT_IN_TXON		(1 << 4)
+#define DA9030_ADC_VCH_ENABLE		(1 << 3)
+#define DA9030_ADC_ICH_ENABLE		(1 << 2)
+#define DA9030_ADC_VBAT_ENABLE		(1 << 1)
+#define DA9030_ADC_AUTO_SLEEP_ENABLE	(1 << 0)
+
+#define DA9030_VBATMON		0x32
+#define DA9030_VBATMONTXON	0x33
+#define DA9030_TBATHIGHP	0x34
+#define DA9030_TBATHIGHN	0x35
+#define DA9030_TBATLOW		0x36
+
+#define DA9030_VBAT_RES		0x41
+#define DA9030_VBATMIN_RES	0x42
+#define DA9030_VBATMINTXON_RES	0x43
+#define DA9030_ICHMAX_RES	0x44
+#define DA9030_ICHMIN_RES	0x45
+#define DA9030_ICHAVERAGE_RES	0x46
+#define DA9030_VCHMAX_RES	0x47
+#define DA9030_VCHMIN_RES	0x48
+#define DA9030_TBAT_RES		0x49
+
+struct da9030_adc_res {
+	uint8_t vbat_res;
+	uint8_t vbatmin_res;
+	uint8_t vbatmintxon;
+	uint8_t ichmax_res;
+	uint8_t ichmin_res;
+	uint8_t ichaverage_res;
+	uint8_t vchmax_res;
+	uint8_t vchmin_res;
+	uint8_t tbat_res;
+	uint8_t adc_in4_res;
+	uint8_t adc_in5_res;
+};
+
+struct da9030_battery_thresholds {
+	int tbat_low;
+	int tbat_high;
+	int tbat_restart;
+
+	int vbat_low;
+	int vbat_crit;
+	int vbat_charge_start;
+	int vbat_charge_stop;
+	int vbat_charge_restart;
+
+	int vcharge_min;
+	int vcharge_max;
+};
+
+struct da9030_charger {
+	struct power_supply psy;
+
+	struct device *master;
+
+	struct da9030_adc_res adc;
+	struct delayed_work work;
+	unsigned int interval;
+
+	struct power_supply_info *battery_info;
+
+	struct da9030_battery_thresholds thresholds;
+
+	unsigned int charge_milliamp;
+	unsigned int charge_millivolt;
+
+	/* charger status */
+	bool chdet;
+	uint8_t fault;
+	int mA;
+	int mV;
+	bool is_on;
+
+	struct notifier_block nb;
+
+	/* platform callbacks for battery low and critical events */
+	void (*battery_low)(void);
+	void (*battery_critical)(void);
+
+	struct dentry *debug_file;
+};
+
+static inline int da9030_reg_to_mV(int reg)
+{
+	return ((reg * 2650) >> 8) + 2650;
+}
+
+static inline int da9030_millivolt_to_reg(int mV)
+{
+	return ((mV - 2650) << 8) / 2650;
+}
+
+static inline int da9030_reg_to_mA(int reg)
+{
+	return ((reg * 24000) >> 8) / 15;
+}
+
+#ifdef CONFIG_DEBUG_FS
+static int bat_debug_show(struct seq_file *s, void *data)
+{
+	struct da9030_charger *charger = s->private;
+
+	seq_printf(s, "charger is %s\n", charger->is_on ? "on" : "off");
+	if (charger->chdet) {
+		seq_printf(s, "iset = %dmA, vset = %dmV\n",
+			   charger->mA, charger->mV);
+	}
+
+	seq_printf(s, "vbat_res = %d (%dmV)\n",
+		   charger->adc.vbat_res,
+		   da9030_reg_to_mV(charger->adc.vbat_res));
+	seq_printf(s, "vbatmin_res = %d (%dmV)\n",
+		   charger->adc.vbatmin_res,
+		   da9030_reg_to_mV(charger->adc.vbatmin_res));
+	seq_printf(s, "vbatmintxon = %d (%dmV)\n",
+		   charger->adc.vbatmintxon,
+		   da9030_reg_to_mV(charger->adc.vbatmintxon));
+	seq_printf(s, "ichmax_res = %d (%dmA)\n",
+		   charger->adc.ichmax_res,
+		   da9030_reg_to_mV(charger->adc.ichmax_res));
+	seq_printf(s, "ichmin_res = %d (%dmA)\n",
+		   charger->adc.ichmin_res,
+		   da9030_reg_to_mA(charger->adc.ichmin_res));
+	seq_printf(s, "ichaverage_res = %d (%dmA)\n",
+		   charger->adc.ichaverage_res,
+		   da9030_reg_to_mA(charger->adc.ichaverage_res));
+	seq_printf(s, "vchmax_res = %d (%dmV)\n",
+		   charger->adc.vchmax_res,
+		   da9030_reg_to_mA(charger->adc.vchmax_res));
+	seq_printf(s, "vchmin_res = %d (%dmV)\n",
+		   charger->adc.vchmin_res,
+		   da9030_reg_to_mV(charger->adc.vchmin_res));
+
+	return 0;
+}
+
+static int debug_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, bat_debug_show, inode->i_private);
+}
+
+static const struct file_operations bat_debug_fops = {
+	.open		= debug_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)
+{
+	charger->debug_file = debugfs_create_file("charger", 0666, 0, charger,
+						 &bat_debug_fops);
+	return charger->debug_file;
+}
+
+static void da9030_bat_remove_debugfs(struct da9030_charger *charger)
+{
+	debugfs_remove(charger->debug_file);
+}
+#else
+static inline struct dentry *da9030_bat_create_debugfs(struct da9030_charger *charger)
+{
+	return NULL;
+}
+static inline void da9030_bat_remove_debugfs(struct da9030_charger *charger)
+{
+}
+#endif
+
+static inline void da9030_read_adc(struct da9030_charger *charger,
+				   struct da9030_adc_res *adc)
+{
+	da903x_reads(charger->master, DA9030_VBAT_RES,
+		     sizeof(*adc), (uint8_t *)adc);
+}
+
+static void da9030_charger_update_state(struct da9030_charger *charger)
+{
+	uint8_t val;
+
+	da903x_read(charger->master, DA9030_CHARGE_CONTROL, &val);
+	charger->is_on = (val & DA9030_CHRG_CHARGER_ENABLE) ? 1 : 0;
+	charger->mA = ((val >> 3) & 0xf) * 100;
+	charger->mV = (val & 0x7) * 50 + 4000;
+
+	da9030_read_adc(charger, &charger->adc);
+	da903x_read(charger->master, DA9030_FAULT_LOG, &charger->fault);
+	charger->chdet = da903x_query_status(charger->master,
+						     DA9030_STATUS_CHDET);
+}
+
+static void da9030_set_charge(struct da9030_charger *charger, int on)
+{
+	uint8_t val;
+
+	if (on) {
+		val = DA9030_CHRG_CHARGER_ENABLE;
+		val |= (charger->charge_milliamp / 100) << 3;
+		val |= (charger->charge_millivolt - 4000) / 50;
+		charger->is_on = 1;
+	} else {
+		val = 0;
+		charger->is_on = 0;
+	}
+
+	da903x_write(charger->master, DA9030_CHARGE_CONTROL, val);
+}
+
+static void da9030_charger_check_state(struct da9030_charger *charger)
+{
+	da9030_charger_update_state(charger);
+
+	/* we wake or boot with external power on */
+	if (!charger->is_on) {
+		if ((charger->chdet) &&
+		    (charger->adc.vbat_res <
+		     charger->thresholds.vbat_charge_start)) {
+			da9030_set_charge(charger, 1);
+		}
+	} else {
+		if (charger->adc.vbat_res >=
+		    charger->thresholds.vbat_charge_stop) {
+			da9030_set_charge(charger, 0);
+			da903x_write(charger->master, DA9030_VBATMON,
+				       charger->thresholds.vbat_charge_restart);
+		} else if (charger->adc.vbat_res >
+			   charger->thresholds.vbat_low) {
+			/* we are charging and passed LOW_THRESH,
+			   so upate DA9030 VBAT threshold
+			 */
+			da903x_write(charger->master, DA9030_VBATMON,
+				     charger->thresholds.vbat_low);
+		}
+		if (charger->adc.vchmax_res > charger->thresholds.vcharge_max ||
+		    charger->adc.vchmin_res < charger->thresholds.vcharge_min ||
+		    /* Tempreture readings are negative */
+		    charger->adc.tbat_res < charger->thresholds.tbat_high ||
+		    charger->adc.tbat_res > charger->thresholds.tbat_low) {
+			/* disable charger */
+			da9030_set_charge(charger, 0);
+		}
+	}
+}
+
+static void da9030_charging_monitor(struct work_struct *work)
+{
+	struct da9030_charger *charger;
+
+	charger = container_of(work, struct da9030_charger, work.work);
+
+	da9030_charger_check_state(charger);
+
+	/* reschedule for the next time */
+	schedule_delayed_work(&charger->work, charger->interval);
+}
+
+static enum power_supply_property da9030_battery_props[] = {
+	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_AVG,
+};
+
+static void da9030_battery_check_status(struct da9030_charger *charger,
+				    union power_supply_propval *val)
+{
+	if (charger->chdet) {
+		if (charger->is_on)
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+		else
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+	} else {
+		val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+	}
+}
+
+static void da9030_battery_check_health(struct da9030_charger *charger,
+				    union power_supply_propval *val)
+{
+	if (charger->fault & DA9030_FAULT_LOG_OVER_TEMP)
+		val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+	else if (charger->fault & DA9030_FAULT_LOG_VBAT_OVER)
+		val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+	else
+		val->intval = POWER_SUPPLY_HEALTH_GOOD;
+}
+
+static int da9030_battery_get_property(struct power_supply *psy,
+				   enum power_supply_property psp,
+				   union power_supply_propval *val)
+{
+	struct da9030_charger *charger;
+	charger = container_of(psy, struct da9030_charger, psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		da9030_battery_check_status(charger, val);
+		break;
+	case POWER_SUPPLY_PROP_HEALTH:
+		da9030_battery_check_health(charger, val);
+		break;
+	case POWER_SUPPLY_PROP_TECHNOLOGY:
+		val->intval = charger->battery_info->technology;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		val->intval = charger->battery_info->voltage_max_design;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		val->intval = charger->battery_info->voltage_min_design;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = da9030_reg_to_mV(charger->adc.vbat_res) * 1000;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_AVG:
+		val->intval =
+			da9030_reg_to_mA(charger->adc.ichaverage_res) * 1000;
+		break;
+	case POWER_SUPPLY_PROP_MODEL_NAME:
+		val->strval = charger->battery_info->name;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static void da9030_battery_vbat_event(struct da9030_charger *charger)
+{
+	da9030_read_adc(charger, &charger->adc);
+
+	if (charger->is_on)
+		return;
+
+	if (charger->adc.vbat_res < charger->thresholds.vbat_low) {
+		/* set VBAT threshold for critical */
+		da903x_write(charger->master, DA9030_VBATMON,
+			     charger->thresholds.vbat_crit);
+		if (charger->battery_low)
+			charger->battery_low();
+	} else if (charger->adc.vbat_res <
+		   charger->thresholds.vbat_crit) {
+		/* notify the system of battery critical */
+		if (charger->battery_critical)
+			charger->battery_critical();
+	}
+}
+
+static int da9030_battery_event(struct notifier_block *nb, unsigned long event,
+				void *data)
+{
+	struct da9030_charger *charger =
+		container_of(nb, struct da9030_charger, nb);
+	int status;
+
+	switch (event) {
+	case DA9030_EVENT_CHDET:
+		status = da903x_query_status(charger->master,
+					     DA9030_STATUS_CHDET);
+		da9030_set_charge(charger, status);
+		break;
+	case DA9030_EVENT_VBATMON:
+		da9030_battery_vbat_event(charger);
+		break;
+	case DA9030_EVENT_CHIOVER:
+	case DA9030_EVENT_TBAT:
+		da9030_set_charge(charger, 0);
+		break;
+	}
+
+	return 0;
+}
+
+static void da9030_battery_convert_thresholds(struct da9030_charger *charger,
+					      struct da9030_battery_info *pdata)
+{
+	charger->thresholds.tbat_low = pdata->tbat_low;
+	charger->thresholds.tbat_high = pdata->tbat_high;
+	charger->thresholds.tbat_restart  = pdata->tbat_restart;
+
+	charger->thresholds.vbat_low =
+		da9030_millivolt_to_reg(pdata->vbat_low);
+	charger->thresholds.vbat_crit =
+		da9030_millivolt_to_reg(pdata->vbat_crit);
+	charger->thresholds.vbat_charge_start =
+		da9030_millivolt_to_reg(pdata->vbat_charge_start);
+	charger->thresholds.vbat_charge_stop =
+		da9030_millivolt_to_reg(pdata->vbat_charge_stop);
+	charger->thresholds.vbat_charge_restart =
+		da9030_millivolt_to_reg(pdata->vbat_charge_restart);
+
+	charger->thresholds.vcharge_min =
+		da9030_millivolt_to_reg(pdata->vcharge_min);
+	charger->thresholds.vcharge_max =
+		da9030_millivolt_to_reg(pdata->vcharge_max);
+}
+
+static void da9030_battery_setup_psy(struct da9030_charger *charger)
+{
+	struct power_supply *psy = &charger->psy;
+	struct power_supply_info *info = charger->battery_info;
+
+	psy->name = info->name;
+	psy->use_for_apm = info->use_for_apm;
+	psy->type = POWER_SUPPLY_TYPE_BATTERY;
+	psy->get_property = da9030_battery_get_property;
+
+	psy->properties = da9030_battery_props;
+	psy->num_properties = ARRAY_SIZE(da9030_battery_props);
+};
+
+static int da9030_battery_charger_init(struct da9030_charger *charger)
+{
+	char v[5];
+	int ret;
+
+	v[0] = v[1] = charger->thresholds.vbat_low;
+	v[2] = charger->thresholds.tbat_high;
+	v[3] = charger->thresholds.tbat_restart;
+	v[4] = charger->thresholds.tbat_low;
+
+	ret = da903x_writes(charger->master, DA9030_VBATMON, 5, v);
+	if (ret)
+		return ret;
+
+	/*
+	 * Enable reference voltage supply for ADC from the LDO_INTERNAL
+	 * regulator. Must be set before ADC measurements can be made.
+	 */
+	ret = da903x_write(charger->master, DA9030_ADC_MAN_CONTROL,
+			   DA9030_ADC_LDO_INT_ENABLE |
+			   DA9030_ADC_TBATREF_ENABLE);
+	if (ret)
+		return ret;
+
+	/* enable auto ADC measuremnts */
+	return da903x_write(charger->master, DA9030_ADC_AUTO_CONTROL,
+			    DA9030_ADC_TBAT_ENABLE | DA9030_ADC_VBAT_IN_TXON |
+			    DA9030_ADC_VCH_ENABLE | DA9030_ADC_ICH_ENABLE |
+			    DA9030_ADC_VBAT_ENABLE |
+			    DA9030_ADC_AUTO_SLEEP_ENABLE);
+}
+
+static int da9030_battery_probe(struct platform_device *pdev)
+{
+	struct da9030_charger *charger;
+	struct da9030_battery_info *pdata = pdev->dev.platform_data;
+	int ret;
+
+	if (pdata == NULL)
+		return -EINVAL;
+
+	if (pdata->charge_milliamp >= 1500 ||
+	    pdata->charge_millivolt < 4000 ||
+	    pdata->charge_millivolt > 4350)
+		return -EINVAL;
+
+	charger = kzalloc(sizeof(*charger), GFP_KERNEL);
+	if (charger == NULL)
+		return -ENOMEM;
+
+	charger->master = pdev->dev.parent;
+
+	/* 10 seconds between monotor runs unless platfrom defines other
+	   interval */
+	charger->interval = msecs_to_jiffies(
+		(pdata->batmon_interval ? : 10) * 1000);
+
+	charger->charge_milliamp = pdata->charge_milliamp;
+	charger->charge_millivolt = pdata->charge_millivolt;
+	charger->battery_info = pdata->battery_info;
+	charger->battery_low = pdata->battery_low;
+	charger->battery_critical = pdata->battery_critical;
+
+	da9030_battery_convert_thresholds(charger, pdata);
+
+	ret = da9030_battery_charger_init(charger);
+	if (ret)
+		goto err_charger_init;
+
+	INIT_DELAYED_WORK(&charger->work, da9030_charging_monitor);
+	schedule_delayed_work(&charger->work, charger->interval);
+
+	charger->nb.notifier_call = da9030_battery_event;
+	ret = da903x_register_notifier(charger->master, &charger->nb,
+				       DA9030_EVENT_CHDET |
+				       DA9030_EVENT_VBATMON |
+				       DA9030_EVENT_CHIOVER |
+				       DA9030_EVENT_TBAT);
+	if (ret)
+		goto err_notifier;
+
+	da9030_battery_setup_psy(charger);
+	ret = power_supply_register(&pdev->dev, &charger->psy);
+	if (ret)
+		goto err_ps_register;
+
+	charger->debug_file = da9030_bat_create_debugfs(charger);
+	platform_set_drvdata(pdev, charger);
+	return 0;
+
+err_ps_register:
+	da903x_unregister_notifier(charger->master, &charger->nb,
+				   DA9030_EVENT_CHDET | DA9030_EVENT_VBATMON |
+				   DA9030_EVENT_CHIOVER | DA9030_EVENT_TBAT);
+err_notifier:
+	cancel_delayed_work(&charger->work);
+
+err_charger_init:
+	kfree(charger);
+
+	return ret;
+}
+
+static int da9030_battery_remove(struct platform_device *dev)
+{
+	struct da9030_charger *charger = platform_get_drvdata(dev);
+
+	da9030_bat_remove_debugfs(charger);
+
+	da903x_unregister_notifier(charger->master, &charger->nb,
+				   DA9030_EVENT_CHDET | DA9030_EVENT_VBATMON |
+				   DA9030_EVENT_CHIOVER | DA9030_EVENT_TBAT);
+	cancel_delayed_work(&charger->work);
+	power_supply_unregister(&charger->psy);
+
+	kfree(charger);
+
+	return 0;
+}
+
+static struct platform_driver da903x_battery_driver = {
+	.driver	= {
+		.name	= "da903x-battery",
+		.owner	= THIS_MODULE,
+	},
+	.probe = da9030_battery_probe,
+	.remove = da9030_battery_remove,
+};
+
+static int da903x_battery_init(void)
+{
+	return platform_driver_register(&da903x_battery_driver);
+}
+
+static void da903x_battery_exit(void)
+{
+	platform_driver_unregister(&da903x_battery_driver);
+}
+
+module_init(da903x_battery_init);
+module_exit(da903x_battery_exit);
+
+MODULE_DESCRIPTION("DA9030 battery charger driver");
+MODULE_AUTHOR("Mike Rapoport, CompuLab");
+MODULE_LICENSE("GPL");
Signature · 3 lines
-- 
Sincerely yours,
Mike.
Signature · 2 lines
Intel Open Source Technology Centre
http://oss.intel.com/