All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver
@ 2008-12-17  7:56 Mike Rapoport
  2008-12-18  6:33 ` Andrew Morton
  2008-12-18 10:17 ` Samuel Ortiz
  0 siblings, 2 replies; 10+ messages in thread
From: Mike Rapoport @ 2008-12-17  7:56 UTC (permalink / raw)
  To: LKML; +Cc: sameo, eric miao, cbou, David Woodhouse, Jonathan Cameron

Driver for battery charger integrated into Dialog Semiconductor DA9030 PMIC



Signed-off-by: Mike Rapoport <mike@compulab.co.il>

 drivers/power/Kconfig          |    7 +
 drivers/power/Makefile         |    1 +
 drivers/power/da9030_battery.c |  592 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 600 insertions(+), 0 deletions(-)

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 8e0c2b4..db8145c 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -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
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index e8f1ece..58d1b46 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -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
diff --git a/drivers/power/da9030_battery.c b/drivers/power/da9030_battery.c
new file mode 100644
index 0000000..2dd9fef
--- /dev/null
+++ b/drivers/power/da9030_battery.c
@@ -0,0 +1,592 @@
+/*
+ * Battery charger driver for Dialog Semiconductor DA9030
+ *
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <mike@compulab.co.il>
+ *
+ * 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");

-- 
Sincerely yours,
Mike.


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver
  2008-12-17  7:56 [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver Mike Rapoport
@ 2008-12-18  6:33 ` Andrew Morton
  2008-12-18 10:17 ` Samuel Ortiz
  1 sibling, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2008-12-18  6:33 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: LKML, sameo, eric miao, cbou, David Woodhouse, Jonathan Cameron

On Wed, 17 Dec 2008 09:56:10 +0200 Mike Rapoport <mike@compulab.co.il> wrote:

> Driver for battery charger integrated into Dialog Semiconductor DA9030 PMIC
> 
> 
> 
> Signed-off-by: Mike Rapoport <mike@compulab.co.il>
> 
>  drivers/power/Kconfig          |    7 +
>  drivers/power/Makefile         |    1 +
>  drivers/power/da9030_battery.c |  592 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 600 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 8e0c2b4..db8145c 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -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
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index e8f1ece..58d1b46 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -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
> diff --git a/drivers/power/da9030_battery.c b/drivers/power/da9030_battery.c
> new file mode 100644
> index 0000000..2dd9fef
> --- /dev/null
> +++ b/drivers/power/da9030_battery.c
> @@ -0,0 +1,592 @@
> +/*
> + * Battery charger driver for Dialog Semiconductor DA9030
> + *
> + * Copyright (C) 2008 Compulab, Ltd.
> + * 	Mike Rapoport <mike@compulab.co.il>
> + *
> + * 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

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.

> +#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;
> +};

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

> +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);
> +	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.

> +#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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver
  2008-12-17  7:56 [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver Mike Rapoport
  2008-12-18  6:33 ` Andrew Morton
@ 2008-12-18 10:17 ` Samuel Ortiz
  2008-12-18 11:36   ` Mike Rapoport
  1 sibling, 1 reply; 10+ messages in thread
From: Samuel Ortiz @ 2008-12-18 10:17 UTC (permalink / raw)
  To: Mike Rapoport; +Cc: LKML, eric miao, cbou, David Woodhouse, Jonathan Cameron

Hi Mike,

On Wed, Dec 17, 2008 at 09:56:10AM +0200, Mike Rapoport wrote:
> 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.

> Signed-off-by: Mike Rapoport <mike@compulab.co.il>
> 
>  drivers/power/Kconfig          |    7 +
>  drivers/power/Makefile         |    1 +
>  drivers/power/da9030_battery.c |  592 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 600 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 8e0c2b4..db8145c 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -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
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index e8f1ece..58d1b46 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -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
> diff --git a/drivers/power/da9030_battery.c b/drivers/power/da9030_battery.c
> new file mode 100644
> index 0000000..2dd9fef
> --- /dev/null
> +++ b/drivers/power/da9030_battery.c
> @@ -0,0 +1,592 @@
> +/*
> + * Battery charger driver for Dialog Semiconductor DA9030
> + *
> + * Copyright (C) 2008 Compulab, Ltd.
> + * 	Mike Rapoport <mike@compulab.co.il>
> + *
> + * 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");
> 
> -- 
> Sincerely yours,
> Mike.
> 

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver
  2008-12-18 10:17 ` Samuel Ortiz
@ 2008-12-18 11:36   ` Mike Rapoport
  2008-12-18 20:55     ` Anton Vorontsov
  0 siblings, 1 reply; 10+ messages in thread
From: Mike Rapoport @ 2008-12-18 11:36 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: LKML, eric miao, cbou, David Woodhouse, Jonathan Cameron, Andrew Morton

Hi Samuel,

Samuel Ortiz wrote:
> Hi Mike,
> 
> On Wed, Dec 17, 2008 at 09:56:10AM +0200, Mike Rapoport wrote:
>> 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 :).

> 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.

> Cheers,
> Samuel.
> 

Signed-off-by: Mike Rapoport <mike@compulab.co.il>

 drivers/power/Kconfig          |    7 +
 drivers/power/Makefile         |    1 +
 drivers/power/da9030_battery.c |  593 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 601 insertions(+), 0 deletions(-)

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 8e0c2b4..afa17b9 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -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
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index e8f1ece..1c83ede 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -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
diff --git a/drivers/power/da9030_battery.c b/drivers/power/da9030_battery.c
new file mode 100644
index 0000000..397375d
--- /dev/null
+++ b/drivers/power/da9030_battery.c
@@ -0,0 +1,593 @@
+/*
+ * Battery charger driver for Dialog Semiconductor DA9030
+ *
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <mike@compulab.co.il>
+ *
+ * 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>
+
+#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 */
+	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);
+
+	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 = 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");


-- 
Sincerely yours,
Mike.


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver
  2008-12-18 11:36   ` Mike Rapoport
@ 2008-12-18 20:55     ` Anton Vorontsov
  2008-12-18 22:08       ` Samuel Ortiz
  0 siblings, 1 reply; 10+ messages in thread
From: Anton Vorontsov @ 2008-12-18 20:55 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Samuel Ortiz, LKML, eric miao, cbou, David Woodhouse,
	Jonathan Cameron, Andrew Morton

Hi all,

On Thu, Dec 18, 2008 at 01:36:28PM +0200, Mike Rapoport wrote:
[...]
> > 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.

Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>

> > 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.

[...]
> +++ b/drivers/power/da9030_battery.c
> @@ -0,0 +1,593 @@
> +/*
> + * Battery charger driver for Dialog Semiconductor DA9030
> + *
> + * Copyright (C) 2008 Compulab, Ltd.
> + * 	Mike Rapoport <mike@compulab.co.il>
> + *
> + * 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.

> +#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 */
> +	int chdet:1;

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

> +	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)
> +{
> +	return ((mV - 2650) << 8) / 2650;
> +}
> +
> +static inline int da9030_reg_to_mA(int reg)
> +{
> +	return ((reg * 24000) >> 8) / 15;
> +}
> +
> +#ifdef CONFIG_DEBUG_FS
> +

(1)

> +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);
> +}

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

> +#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 = 0;

= 0 isn't necessary here.

> +
> +	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) {

I'd write it as:

if (charger->is_on)
	return;
the-rest;

Saves one indent level.

> +		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);;

One semicolon is enough.

> +};
> +
> +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. */

I would suggest using canonical-style comments:

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

Thanks,

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver
  2008-12-18 20:55     ` Anton Vorontsov
@ 2008-12-18 22:08       ` Samuel Ortiz
  2008-12-21  8:29         ` Mike Rapoport
  0 siblings, 1 reply; 10+ messages in thread
From: Samuel Ortiz @ 2008-12-18 22:08 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Mike Rapoport, LKML, eric miao, cbou, David Woodhouse,
	Jonathan Cameron, Andrew Morton

On Thu, Dec 18, 2008 at 11:55:50PM +0300, Anton Vorontsov wrote:
> Hi all,
> 
> On Thu, Dec 18, 2008 at 01:36:28PM +0200, Mike Rapoport wrote:
> [...]
> > > 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.
> 
> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
Thanks Anton, I just applied Mike's latest version of the patch.

Cheers,
Samuel.

 
> > > 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.
> 
> [...]
> > +++ b/drivers/power/da9030_battery.c
> > @@ -0,0 +1,593 @@
> > +/*
> > + * Battery charger driver for Dialog Semiconductor DA9030
> > + *
> > + * Copyright (C) 2008 Compulab, Ltd.
> > + * 	Mike Rapoport <mike@compulab.co.il>
> > + *
> > + * 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.
> 
> > +#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 */
> > +	int chdet:1;
> 
> In kernel we can use bool type, and true/false constants.
> 
> > +	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)
> > +{
> > +	return ((mV - 2650) << 8) / 2650;
> > +}
> > +
> > +static inline int da9030_reg_to_mA(int reg)
> > +{
> > +	return ((reg * 24000) >> 8) / 15;
> > +}
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > +
> 
> (1)
> 
> > +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);
> > +}
> 
> I'd put an empty line here, or remove empty line at (1). Just for
> consistency.
> 
> > +#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 = 0;
> 
> = 0 isn't necessary here.
> 
> > +
> > +	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) {
> 
> I'd write it as:
> 
> if (charger->is_on)
> 	return;
> the-rest;
> 
> Saves one indent level.
> 
> > +		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);;
> 
> One semicolon is enough.
> 
> > +};
> > +
> > +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. */
> 
> I would suggest using canonical-style comments:
> 
> /*
>  * Not a big deal though, I'm
>  * just nitpicking. ;-)
>  */
> 
> Thanks,
> 
> -- 
> Anton Vorontsov
> email: cbouatmailru@gmail.com
> irc://irc.freenode.net/bd2

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver
  2008-12-18 22:08       ` Samuel Ortiz
@ 2008-12-21  8:29         ` Mike Rapoport
  2008-12-22 10:22           ` Samuel Ortiz
  0 siblings, 1 reply; 10+ messages in thread
From: Mike Rapoport @ 2008-12-21  8:29 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: Anton Vorontsov, LKML, eric miao, cbou, David Woodhouse,
	Jonathan Cameron, Andrew Morton

Samuel,

Samuel Ortiz wrote:
> On Thu, Dec 18, 2008 at 11:55:50PM +0300, Anton Vorontsov wrote:
>> Hi all,
>>
>> On Thu, Dec 18, 2008 at 01:36:28PM +0200, Mike Rapoport wrote:
>> [...]
>>>> 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.
>>
>> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
> 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?

> Cheers,
> Samuel.
> 
>  
>>>> 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.
>>
>> [...]
>>> +++ b/drivers/power/da9030_battery.c
>>> @@ -0,0 +1,593 @@
>>> +/*
>>> + * Battery charger driver for Dialog Semiconductor DA9030
>>> + *
>>> + * Copyright (C) 2008 Compulab, Ltd.
>>> + * 	Mike Rapoport <mike@compulab.co.il>
>>> + *
>>> + * 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.
>>
>>> +#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 */
>>> +	int chdet:1;
>> In kernel we can use bool type, and true/false constants.
>>
>>> +	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)
>>> +{
>>> +	return ((mV - 2650) << 8) / 2650;
>>> +}
>>> +
>>> +static inline int da9030_reg_to_mA(int reg)
>>> +{
>>> +	return ((reg * 24000) >> 8) / 15;
>>> +}
>>> +
>>> +#ifdef CONFIG_DEBUG_FS
>>> +
>> (1)
>>
>>> +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);
>>> +}
>> I'd put an empty line here, or remove empty line at (1). Just for
>> consistency.
>>
>>> +#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 = 0;
>> = 0 isn't necessary here.
>>
>>> +
>>> +	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) {
>> I'd write it as:
>>
>> if (charger->is_on)
>> 	return;
>> the-rest;
>>
>> Saves one indent level.
>>
>>> +		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);;
>> One semicolon is enough.
>>
>>> +};
>>> +
>>> +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. */
>> I would suggest using canonical-style comments:
>>
>> /*
>>  * Not a big deal though, I'm
>>  * just nitpicking. ;-)
>>  */
>>
>> Thanks,
>>
>> -- 
>> Anton Vorontsov
>> email: cbouatmailru@gmail.com
>> irc://irc.freenode.net/bd2
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver
  2008-12-21  8:29         ` Mike Rapoport
@ 2008-12-22 10:22           ` Samuel Ortiz
  2008-12-22 12:03             ` Mike Rapoport
  0 siblings, 1 reply; 10+ messages in thread
From: Samuel Ortiz @ 2008-12-22 10:22 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Anton Vorontsov, LKML, eric miao, cbou, David Woodhouse,
	Jonathan Cameron, Andrew Morton

On Sun, Dec 21, 2008 at 10:29:01AM +0200, Mike Rapoport wrote:
> Samuel,
> 
> Samuel Ortiz wrote:
> > On Thu, Dec 18, 2008 at 11:55:50PM +0300, Anton Vorontsov wrote:
> >> Hi all,
> >>
> >> On Thu, Dec 18, 2008 at 01:36:28PM +0200, Mike Rapoport wrote:
> >> [...]
> >>>> 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.
> >>
> >> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
> > 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.

 
> > Cheers,
> > Samuel.
> > 
> >  
> >>>> 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.
> >>
> >> [...]
> >>> +++ b/drivers/power/da9030_battery.c
> >>> @@ -0,0 +1,593 @@
> >>> +/*
> >>> + * Battery charger driver for Dialog Semiconductor DA9030
> >>> + *
> >>> + * Copyright (C) 2008 Compulab, Ltd.
> >>> + * 	Mike Rapoport <mike@compulab.co.il>
> >>> + *
> >>> + * 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.
> >>
> >>> +#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 */
> >>> +	int chdet:1;
> >> In kernel we can use bool type, and true/false constants.
> >>
> >>> +	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)
> >>> +{
> >>> +	return ((mV - 2650) << 8) / 2650;
> >>> +}
> >>> +
> >>> +static inline int da9030_reg_to_mA(int reg)
> >>> +{
> >>> +	return ((reg * 24000) >> 8) / 15;
> >>> +}
> >>> +
> >>> +#ifdef CONFIG_DEBUG_FS
> >>> +
> >> (1)
> >>
> >>> +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);
> >>> +}
> >> I'd put an empty line here, or remove empty line at (1). Just for
> >> consistency.
> >>
> >>> +#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 = 0;
> >> = 0 isn't necessary here.
> >>
> >>> +
> >>> +	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) {
> >> I'd write it as:
> >>
> >> if (charger->is_on)
> >> 	return;
> >> the-rest;
> >>
> >> Saves one indent level.
> >>
> >>> +		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);;
> >> One semicolon is enough.
> >>
> >>> +};
> >>> +
> >>> +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. */
> >> I would suggest using canonical-style comments:
> >>
> >> /*
> >>  * Not a big deal though, I'm
> >>  * just nitpicking. ;-)
> >>  */
> >>
> >> Thanks,
> >>
> >> -- 
> >> Anton Vorontsov
> >> email: cbouatmailru@gmail.com
> >> irc://irc.freenode.net/bd2
> > 
> 
> -- 
> Sincerely yours,
> Mike.
> 

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver
  2008-12-22 10:22           ` Samuel Ortiz
@ 2008-12-22 12:03             ` Mike Rapoport
  2008-12-30 22:41               ` Samuel Ortiz
  0 siblings, 1 reply; 10+ messages in thread
From: Mike Rapoport @ 2008-12-22 12:03 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: Anton Vorontsov, LKML, eric miao, cbou, David Woodhouse,
	Jonathan Cameron, Andrew Morton

Samuel Ortiz wrote:
> On Sun, Dec 21, 2008 at 10:29:01AM +0200, Mike Rapoport wrote:
>> Samuel,
>>
>> Samuel Ortiz wrote:
>>> On Thu, Dec 18, 2008 at 11:55:50PM +0300, Anton Vorontsov wrote:
>>>> Hi all,
>>>>
>>>> On Thu, Dec 18, 2008 at 01:36:28PM +0200, Mike Rapoport wrote:
>>>> [...]
>>>>>> 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.
>>>>
>>>> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
>>> 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.

> Cheers,
> Samuel.

Signed-off-by: Mike Rapoport <mike@compulab.co.il>
Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>

 drivers/power/Kconfig          |    7 +
 drivers/power/Makefile         |    1 +
 drivers/power/da9030_battery.c |  600 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 608 insertions(+), 0 deletions(-)

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 8e0c2b4..afa17b9 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -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
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index e8f1ece..1c83ede 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -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
diff --git a/drivers/power/da9030_battery.c b/drivers/power/da9030_battery.c
new file mode 100644
index 0000000..1662bb0
--- /dev/null
+++ b/drivers/power/da9030_battery.c
@@ -0,0 +1,600 @@
+/*
+ * Battery charger driver for Dialog Semiconductor DA9030
+ *
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <mike@compulab.co.il>
+ *
+ * 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");


-- 
Sincerely yours,
Mike.



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver
  2008-12-22 12:03             ` Mike Rapoport
@ 2008-12-30 22:41               ` Samuel Ortiz
  0 siblings, 0 replies; 10+ messages in thread
From: Samuel Ortiz @ 2008-12-30 22:41 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Anton Vorontsov, LKML, eric miao, cbou, David Woodhouse,
	Jonathan Cameron, Andrew Morton

On Mon, Dec 22, 2008 at 02:03:33PM +0200, Mike Rapoport wrote:
> Samuel Ortiz wrote:
> > On Sun, Dec 21, 2008 at 10:29:01AM +0200, Mike Rapoport wrote:
> >> Samuel,
> >>
> >> Samuel Ortiz wrote:
> >>> On Thu, Dec 18, 2008 at 11:55:50PM +0300, Anton Vorontsov wrote:
> >>>> Hi all,
> >>>>
> >>>> On Thu, Dec 18, 2008 at 01:36:28PM +0200, Mike Rapoport wrote:
> >>>> [...]
> >>>>>> 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.
> >>>>
> >>>> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
> >>> 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.

 
> > Cheers,
> > Samuel.
> 
> Signed-off-by: Mike Rapoport <mike@compulab.co.il>
> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
> 
>  drivers/power/Kconfig          |    7 +
>  drivers/power/Makefile         |    1 +
>  drivers/power/da9030_battery.c |  600 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 608 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 8e0c2b4..afa17b9 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -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
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index e8f1ece..1c83ede 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -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
> diff --git a/drivers/power/da9030_battery.c b/drivers/power/da9030_battery.c
> new file mode 100644
> index 0000000..1662bb0
> --- /dev/null
> +++ b/drivers/power/da9030_battery.c
> @@ -0,0 +1,600 @@
> +/*
> + * Battery charger driver for Dialog Semiconductor DA9030
> + *
> + * Copyright (C) 2008 Compulab, Ltd.
> + * 	Mike Rapoport <mike@compulab.co.il>
> + *
> + * 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");
> 
> 
> -- 
> Sincerely yours,
> Mike.
> 
> 

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2008-12-30 22:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-17  7:56 [RFC PATCH 2/2] Add Dialog DA9030 battery charger driver Mike Rapoport
2008-12-18  6:33 ` Andrew Morton
2008-12-18 10:17 ` Samuel Ortiz
2008-12-18 11:36   ` Mike Rapoport
2008-12-18 20:55     ` Anton Vorontsov
2008-12-18 22:08       ` Samuel Ortiz
2008-12-21  8:29         ` Mike Rapoport
2008-12-22 10:22           ` Samuel Ortiz
2008-12-22 12:03             ` Mike Rapoport
2008-12-30 22:41               ` Samuel Ortiz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.