From 2e83a5c5d2317c386de2880eb43ef0bef8eb1fa9 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Wed, 15 Jul 2009 18:20:38 +0200 Subject: ds2760_battery: delay power supply registration This fixes a race condition I recently introduced with the PMOD feature addition (cef437e3: "w1: ds2760_battery: add support for sleep mode feature"). Postpone the call to power_supply_register() to fix it. Signed-off-by: Daniel Mack Cc: Szabolcs Gyurko Acked-by: Matt Reimer Signed-off-by: Anton Vorontsov --- drivers/power/ds2760_battery.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers/power/ds2760_battery.c') diff --git a/drivers/power/ds2760_battery.c b/drivers/power/ds2760_battery.c index 520b5c49ff3..cf07c43e90c 100644 --- a/drivers/power/ds2760_battery.c +++ b/drivers/power/ds2760_battery.c @@ -381,12 +381,6 @@ static int ds2760_battery_probe(struct platform_device *pdev) di->charge_status = POWER_SUPPLY_STATUS_UNKNOWN; - retval = power_supply_register(&pdev->dev, &di->bat); - if (retval) { - dev_err(di->dev, "failed to register battery\n"); - goto batt_failed; - } - /* enable sleep mode feature */ ds2760_battery_read_status(di); status = di->raw[DS2760_STATUS_REG]; @@ -397,6 +391,12 @@ static int ds2760_battery_probe(struct platform_device *pdev) ds2760_battery_write_status(di, status); + retval = power_supply_register(&pdev->dev, &di->bat); + if (retval) { + dev_err(di->dev, "failed to register battery\n"); + goto batt_failed; + } + INIT_DELAYED_WORK(&di->monitor_work, ds2760_battery_work); di->monitor_wqueue = create_singlethread_workqueue(dev_name(&pdev->dev)); if (!di->monitor_wqueue) { -- cgit v1.2.3 From 5c6e9bf2c96e746237516bc8897add67682ee452 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Wed, 15 Jul 2009 18:20:39 +0200 Subject: ds2760_battery: export more features Export POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW and POWER_SUPPLY_PROP_CAPACITY features to the power supply core. Signed-off-by: Daniel Mack Cc: Szabolcs Gyurko Acked-by: Matt Reimer Signed-off-by: Anton Vorontsov --- drivers/power/ds2760_battery.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers/power/ds2760_battery.c') diff --git a/drivers/power/ds2760_battery.c b/drivers/power/ds2760_battery.c index cf07c43e90c..f439071c2aa 100644 --- a/drivers/power/ds2760_battery.c +++ b/drivers/power/ds2760_battery.c @@ -337,6 +337,12 @@ static int ds2760_battery_get_property(struct power_supply *psy, case POWER_SUPPLY_PROP_TEMP: val->intval = di->temp_C; break; + case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW: + val->intval = di->life_sec; + break; + case POWER_SUPPLY_PROP_CAPACITY: + val->intval = di->rem_capacity; + break; default: return -EINVAL; } @@ -353,6 +359,8 @@ static enum power_supply_property ds2760_battery_props[] = { POWER_SUPPLY_PROP_CHARGE_EMPTY, POWER_SUPPLY_PROP_CHARGE_NOW, POWER_SUPPLY_PROP_TEMP, + POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW, + POWER_SUPPLY_PROP_CAPACITY, }; static int ds2760_battery_probe(struct platform_device *pdev) -- cgit v1.2.3 From c1e72193ea3fa02e96bf3aa66006e18d107d0266 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Wed, 15 Jul 2009 18:20:40 +0200 Subject: ds2760_battery: add rated_capacity module parameter For systems where the ds2760 is soldered directly on the PCB, the 'rated capacity' register is not set to anything useful. In order to allow users to bootstrap this value, introduce a new module parameter 'rated_capacity' and use it to write the internal EEPROM in case the value differes from what's been given. Signed-off-by: Daniel Mack Cc: Szabolcs Gyurko Acked-by: Matt Reimer Signed-off-by: Anton Vorontsov --- drivers/power/ds2760_battery.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'drivers/power/ds2760_battery.c') diff --git a/drivers/power/ds2760_battery.c b/drivers/power/ds2760_battery.c index f439071c2aa..ed0ea5e275c 100644 --- a/drivers/power/ds2760_battery.c +++ b/drivers/power/ds2760_battery.c @@ -66,6 +66,10 @@ static unsigned int pmod_enabled; module_param(pmod_enabled, bool, 0644); MODULE_PARM_DESC(pmod_enabled, "PMOD enable bit"); +static unsigned int rated_capacity; +module_param(rated_capacity, uint, 0644); +MODULE_PARM_DESC(rated_capacity, "rated battery capacity, 10*mAh or index"); + /* Some batteries have their rated capacity stored a N * 10 mAh, while * others use an index into this table. */ static int rated_capacities[] = { @@ -274,6 +278,17 @@ static void ds2760_battery_write_status(struct ds2760_device_info *di, w1_ds2760_recall_eeprom(di->w1_dev, DS2760_EEPROM_BLOCK1); } +static void ds2760_battery_write_rated_capacity(struct ds2760_device_info *di, + unsigned char rated_capacity) +{ + if (rated_capacity == di->raw[DS2760_RATED_CAPACITY]) + return; + + w1_ds2760_write(di->w1_dev, &rated_capacity, DS2760_RATED_CAPACITY, 1); + w1_ds2760_store_eeprom(di->w1_dev, DS2760_EEPROM_BLOCK1); + w1_ds2760_recall_eeprom(di->w1_dev, DS2760_EEPROM_BLOCK1); +} + static void ds2760_battery_work(struct work_struct *work) { struct ds2760_device_info *di = container_of(work, @@ -399,6 +414,10 @@ static int ds2760_battery_probe(struct platform_device *pdev) ds2760_battery_write_status(di, status); + /* set rated capacity from module param */ + if (rated_capacity) + ds2760_battery_write_rated_capacity(di, rated_capacity); + retval = power_supply_register(&pdev->dev, &di->bat); if (retval) { dev_err(di->dev, "failed to register battery\n"); -- cgit v1.2.3 From 25f2bfa62ae77820a8185734c4a2ab7f3971a2fc Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Wed, 15 Jul 2009 18:20:41 +0200 Subject: ds2760_battery: handle full_active_uAh == 0 case correctly In systems where the battery monitor is not part of the battery pack and is hence not bootstrapped with sane values, the full_active_uAh is likely to be zero. Handle that case by defaulting to the rated_capacity information which can be passed to the driver using the new module parameter. Signed-off-by: Daniel Mack Cc: Szabolcs Gyurko Acked-by: Matt Reimer Signed-off-by: Anton Vorontsov --- drivers/power/ds2760_battery.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'drivers/power/ds2760_battery.c') diff --git a/drivers/power/ds2760_battery.c b/drivers/power/ds2760_battery.c index ed0ea5e275c..2d0e5edb474 100644 --- a/drivers/power/ds2760_battery.c +++ b/drivers/power/ds2760_battery.c @@ -172,8 +172,13 @@ static int ds2760_battery_read_status(struct ds2760_device_info *di) di->full_active_uAh = di->raw[DS2760_ACTIVE_FULL] << 8 | di->raw[DS2760_ACTIVE_FULL + 1]; - scale[0] = di->raw[DS2760_ACTIVE_FULL] << 8 | - di->raw[DS2760_ACTIVE_FULL + 1]; + /* If the full_active_uAh value is not given, fall back to the rated + * capacity. This is likely to happen when chips are not part of the + * battery pack and is therefore not bootstrapped. */ + if (di->full_active_uAh == 0) + di->full_active_uAh = di->rated_capacity / 1000L; + + scale[0] = di->full_active_uAh; for (i = 1; i < 5; i++) scale[i] = scale[i - 1] + di->raw[DS2760_ACTIVE_FULL + 2 + i]; -- cgit v1.2.3 From 02d0d2758821c38b2601d34dac544140af09e651 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Wed, 15 Jul 2009 22:57:16 +0200 Subject: ds2760_battery: add current_accum module parameter When connecting a ds2760 to a partly loaded battery the first time, there must be a way to bootstrap the current_accum value. Without that, the current capactity value is bogus until the battery is fully charged for the first time. Signed-off-by: Daniel Mack Cc: Szabolcs Gyurko Cc: Matt Reimer Signed-off-by: Anton Vorontsov --- drivers/power/ds2760_battery.c | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) (limited to 'drivers/power/ds2760_battery.c') diff --git a/drivers/power/ds2760_battery.c b/drivers/power/ds2760_battery.c index 2d0e5edb474..f4a9258aa9d 100644 --- a/drivers/power/ds2760_battery.c +++ b/drivers/power/ds2760_battery.c @@ -70,6 +70,10 @@ static unsigned int rated_capacity; module_param(rated_capacity, uint, 0644); MODULE_PARM_DESC(rated_capacity, "rated battery capacity, 10*mAh or index"); +static unsigned int current_accum; +module_param(current_accum, uint, 0644); +MODULE_PARM_DESC(current_accum, "current accumulator value"); + /* Some batteries have their rated capacity stored a N * 10 mAh, while * others use an index into this table. */ static int rated_capacities[] = { @@ -215,6 +219,22 @@ static int ds2760_battery_read_status(struct ds2760_device_info *di) return 0; } +static void ds2760_battery_set_current_accum(struct ds2760_device_info *di, + unsigned int acr_val) +{ + unsigned char acr[2]; + + /* acr is in units of 0.25 mAh */ + acr_val *= 4L; + acr_val /= 1000; + + acr[0] = acr_val >> 8; + acr[1] = acr_val & 0xff; + + if (w1_ds2760_write(di->w1_dev, acr, DS2760_CURRENT_ACCUM_MSB, 2) < 2) + dev_warn(di->dev, "ACR write failed\n"); +} + static void ds2760_battery_update_status(struct ds2760_device_info *di) { int old_charge_status = di->charge_status; @@ -246,21 +266,9 @@ static void ds2760_battery_update_status(struct ds2760_device_info *di) if (di->full_counter < 2) { di->charge_status = POWER_SUPPLY_STATUS_CHARGING; } else { - unsigned char acr[2]; - int acr_val; - - /* acr is in units of 0.25 mAh */ - acr_val = di->full_active_uAh * 4L / 1000; - - acr[0] = acr_val >> 8; - acr[1] = acr_val & 0xff; - - if (w1_ds2760_write(di->w1_dev, acr, - DS2760_CURRENT_ACCUM_MSB, 2) < 2) - dev_warn(di->dev, - "ACR reset failed\n"); - di->charge_status = POWER_SUPPLY_STATUS_FULL; + ds2760_battery_set_current_accum(di, + di->full_active_uAh); } } } else { @@ -423,6 +431,11 @@ static int ds2760_battery_probe(struct platform_device *pdev) if (rated_capacity) ds2760_battery_write_rated_capacity(di, rated_capacity); + /* set current accumulator if given as parameter. + * this should only be done for bootstrapping the value */ + if (current_accum) + ds2760_battery_set_current_accum(di, current_accum); + retval = power_supply_register(&pdev->dev, &di->bat); if (retval) { dev_err(di->dev, "failed to register battery\n"); -- cgit v1.2.3 From 8d631ccff8d90fce77b42f01b3872595c599cbf9 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Thu, 23 Jul 2009 20:35:54 +0200 Subject: ds2760_battery: implement set_charged() feature The ds2760's internal current meter is not reliable enough as it has an inacurracy of around ~15%. Without any correction for that error, the current accumulator is couting up all the time, even though the battery is already fully charged and hence destroys the static information. The longer it is connected, the worse is the aberration. Fortunately, this can be corrected by the DS2760_CURRENT_OFFSET_BIAS register. Using the external power_supply_set_battery_charged() function, this register is now gauging the measurement. A delayed work is used to debounce flaky GPIO signals and to let the current value settle. Also see Maxim's application note AN4188. Signed-off-by: Daniel Mack Cc: Szabolcs Gyurko Cc: Matt Reimer Cc: Anton Vorontsov Signed-off-by: Anton Vorontsov --- drivers/power/ds2760_battery.c | 52 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) (limited to 'drivers/power/ds2760_battery.c') diff --git a/drivers/power/ds2760_battery.c b/drivers/power/ds2760_battery.c index f4a9258aa9d..1bb8498f14b 100644 --- a/drivers/power/ds2760_battery.c +++ b/drivers/power/ds2760_battery.c @@ -56,6 +56,7 @@ struct ds2760_device_info { struct device *w1_dev; struct workqueue_struct *monitor_wqueue; struct delayed_work monitor_work; + struct delayed_work set_charged_work; }; static unsigned int cache_time = 1000; @@ -327,6 +328,52 @@ static void ds2760_battery_external_power_changed(struct power_supply *psy) queue_delayed_work(di->monitor_wqueue, &di->monitor_work, HZ/10); } + +static void ds2760_battery_set_charged_work(struct work_struct *work) +{ + char bias; + struct ds2760_device_info *di = container_of(work, + struct ds2760_device_info, set_charged_work.work); + + dev_dbg(di->dev, "%s\n", __func__); + + ds2760_battery_read_status(di); + + /* When we get notified by external circuitry that the battery is + * considered fully charged now, we know that there is no current + * flow any more. However, the ds2760's internal current meter is + * too inaccurate to rely on - spec say something ~15% failure. + * Hence, we use the current offset bias register to compensate + * that error. + */ + + if (!power_supply_am_i_supplied(&di->bat)) + return; + + bias = (signed char) di->current_raw + + (signed char) di->raw[DS2760_CURRENT_OFFSET_BIAS]; + + dev_dbg(di->dev, "%s: bias = %d\n", __func__, bias); + + w1_ds2760_write(di->w1_dev, &bias, DS2760_CURRENT_OFFSET_BIAS, 1); + w1_ds2760_store_eeprom(di->w1_dev, DS2760_EEPROM_BLOCK1); + w1_ds2760_recall_eeprom(di->w1_dev, DS2760_EEPROM_BLOCK1); + + /* Write to the di->raw[] buffer directly - the CURRENT_OFFSET_BIAS + * value won't be read back by ds2760_battery_read_status() */ + di->raw[DS2760_CURRENT_OFFSET_BIAS] = bias; +} + +static void ds2760_battery_set_charged(struct power_supply *psy) +{ + struct ds2760_device_info *di = to_ds2760_device_info(psy); + + /* postpone the actual work by 20 secs. This is for debouncing GPIO + * signals and to let the current value settle. See AN4188. */ + cancel_delayed_work(&di->set_charged_work); + queue_delayed_work(di->monitor_wqueue, &di->set_charged_work, HZ * 20); +} + static int ds2760_battery_get_property(struct power_supply *psy, enum power_supply_property psp, union power_supply_propval *val) @@ -412,6 +459,7 @@ static int ds2760_battery_probe(struct platform_device *pdev) di->bat.properties = ds2760_battery_props; di->bat.num_properties = ARRAY_SIZE(ds2760_battery_props); di->bat.get_property = ds2760_battery_get_property; + di->bat.set_charged = ds2760_battery_set_charged; di->bat.external_power_changed = ds2760_battery_external_power_changed; @@ -443,6 +491,8 @@ static int ds2760_battery_probe(struct platform_device *pdev) } INIT_DELAYED_WORK(&di->monitor_work, ds2760_battery_work); + INIT_DELAYED_WORK(&di->set_charged_work, + ds2760_battery_set_charged_work); di->monitor_wqueue = create_singlethread_workqueue(dev_name(&pdev->dev)); if (!di->monitor_wqueue) { retval = -ESRCH; @@ -467,6 +517,8 @@ static int ds2760_battery_remove(struct platform_device *pdev) cancel_rearming_delayed_workqueue(di->monitor_wqueue, &di->monitor_work); + cancel_rearming_delayed_workqueue(di->monitor_wqueue, + &di->set_charged_work); destroy_workqueue(di->monitor_wqueue); power_supply_unregister(&di->bat); -- cgit v1.2.3 From b0525b48f06714e8d5cf6a3266261b71de8d6dd4 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Fri, 24 Jul 2009 15:08:11 +0200 Subject: ds2760_battery: Fix integer overflow for time_to_empty_now On the device we're currently developing, battery sizes of ~2.8Ah and current flow of ~600mA are typical. With that values, the life_sec computation overflows due to the multiplication by 3600. Signed-off-by: Daniel Mack Cc: Szabolcs Gyurko Cc: Matt Reimer Signed-off-by: Anton Vorontsov --- drivers/power/ds2760_battery.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/power/ds2760_battery.c') diff --git a/drivers/power/ds2760_battery.c b/drivers/power/ds2760_battery.c index 1bb8498f14b..6f1dba5a519 100644 --- a/drivers/power/ds2760_battery.c +++ b/drivers/power/ds2760_battery.c @@ -211,9 +211,9 @@ static int ds2760_battery_read_status(struct ds2760_device_info *di) if (di->rem_capacity > 100) di->rem_capacity = 100; - if (di->current_uA) - di->life_sec = -((di->accum_current_uAh - di->empty_uAh) * - 3600L) / di->current_uA; + if (di->current_uA >= 100L) + di->life_sec = -((di->accum_current_uAh - di->empty_uAh) * 36L) + / (di->current_uA / 100L); else di->life_sec = 0; -- cgit v1.2.3