[PATCH 0/4] power: bq20z75: update support

[PATCH 0/4] power: bq20z75: update support

[rklein]

From: Rhyland Klein <rklein@nvidia.com>

Adds support to driver for the following:
* optional gpio for battery detection
* optional retries for i2c I/O failures
* POWER_NOW property

For more detailed information, see specifics patches.

Rhyland Klein (4):
  power: bq20z75: add optional battery detect gpio
  power: bq20z75: add i2c retry mechanism
  power: bq20z75: add support for POWER_NOW
  power_supply: update power_supply_is_watt_property

 drivers/power/bq20z75.c       |  226 ++++++++++++++++++++++++++++++++++-------
 include/linux/power/bq20z75.h |   39 +++++++
 include/linux/power_supply.h  |    1 +
 3 files changed, 230 insertions(+), 36 deletions(-)
 create mode 100644 include/linux/power/bq20z75.h

[PATCH 1/4] power: bq20z75: add optional battery detect gpio

[rklein]

From: Rhyland Klein <rklein@nvidia.com>

Adding support for an optional gpio for battery detection. This is passed in
through the i2c platform data. It also accepts another field,
battery_detect_present to signify the gpio state which means the battery is
present, either 0 (low) or 1 (high).

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/power/bq20z75.c       |  160 +++++++++++++++++++++++++++++++++--------
 include/linux/power/bq20z75.h |   37 ++++++++++
 2 files changed, 166 insertions(+), 31 deletions(-)
 create mode 100644 include/linux/power/bq20z75.h

diff --git a/drivers/power/bq20z75.c b/drivers/power/bq20z75.c
index 4141775..a51e98d 100644
--- a/drivers/power/bq20z75.c
+++ b/drivers/power/bq20z75.c
@@ -25,6 +25,10 @@
 #include <linux/power_supply.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/gpio.h>
+
+#include <linux/power/bq20z75.h>
 
 enum {
 	REG_MANUFACTURER_DATA,
@@ -141,8 +145,13 @@ static enum power_supply_property bq20z75_properties[] = {
 };
 
 struct bq20z75_info {
-	struct i2c_client	*client;
-	struct power_supply	power_supply;
+	struct i2c_client		*client;
+	struct power_supply		power_supply;
+	struct bq20z75_platform_data	*pdata;
+	bool				is_present;
+	bool				gpio_detect;
+	bool				enable_detection;
+	int				irq;
 };
 
 static int bq20z75_read_word_data(struct i2c_client *client, u8 address)
@@ -179,6 +188,18 @@ static int bq20z75_get_battery_presence_and_health(
 	union power_supply_propval *val)
 {
 	s32 ret;
+	struct bq20z75_info *bq20z75_device = i2c_get_clientdata(client);
+
+	if (psp == POWER_SUPPLY_PROP_PRESENT &&
+		bq20z75_device->gpio_detect) {
+		ret = gpio_get_value(
+			bq20z75_device->pdata->battery_detect);
+		if (ret == bq20z75_device->pdata->battery_detect_present)
+			val->intval = 1;
+		else
+			val->intval = 0;
+		return ret;
+	}
 
 	/* Write to ManufacturerAccess with
 	 * ManufacturerAccess command and then
@@ -192,8 +213,11 @@ static int bq20z75_get_battery_presence_and_health(
 
 	ret = bq20z75_read_word_data(client,
 		bq20z75_data[REG_MANUFACTURER_DATA].addr);
-	if (ret < 0)
+	if (ret < 0) {
+		if (psp == POWER_SUPPLY_PROP_PRESENT)
+			val->intval = 0; /* battery removed */
 		return ret;
+	}
 
 	if (ret < bq20z75_data[REG_MANUFACTURER_DATA].min_value ||
 	    ret > bq20z75_data[REG_MANUFACTURER_DATA].max_value) {
@@ -397,8 +421,7 @@ static int bq20z75_get_property(struct power_supply *psy,
 	enum power_supply_property psp,
 	union power_supply_propval *val)
 {
-	int ps_index;
-	int ret;
+	int ret = 0;
 	struct bq20z75_info *bq20z75_device = container_of(psy,
 				struct bq20z75_info, power_supply);
 	struct i2c_client *client = bq20z75_device->client;
@@ -407,8 +430,6 @@ static int bq20z75_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_PRESENT:
 	case POWER_SUPPLY_PROP_HEALTH:
 		ret = bq20z75_get_battery_presence_and_health(client, psp, val);
-		if (ret)
-			return ret;
 		break;
 
 	case POWER_SUPPLY_PROP_TECHNOLOGY:
@@ -422,20 +443,15 @@ static int bq20z75_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CHARGE_FULL:
 	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
 	case POWER_SUPPLY_PROP_CAPACITY:
-		ps_index = bq20z75_get_property_index(client, psp);
-		if (ps_index < 0)
-			return ps_index;
-
-		ret = bq20z75_get_battery_capacity(client, ps_index, psp, val);
-		if (ret)
-			return ret;
+		ret = bq20z75_get_property_index(client, psp);
+		if (ret < 0)
+			break;
 
+		ret = bq20z75_get_battery_capacity(client, ret, psp, val);
 		break;
 
 	case POWER_SUPPLY_PROP_SERIAL_NUMBER:
 		ret = bq20z75_get_battery_serial_number(client, val);
-		if (ret)
-			return ret;
 		break;
 
 	case POWER_SUPPLY_PROP_STATUS:
@@ -446,14 +462,11 @@ static int bq20z75_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
 	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
 	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
-		ps_index = bq20z75_get_property_index(client, psp);
-		if (ps_index < 0)
-			return ps_index;
-
-		ret = bq20z75_get_battery_property(client, ps_index, psp, val);
-		if (ret)
-			return ret;
+		ret = bq20z75_get_property_index(client, psp);
+		if (ret < 0)
+			break;
 
+		ret = bq20z75_get_battery_property(client, ret, psp, val);
 		break;
 
 	default:
@@ -462,26 +475,51 @@ static int bq20z75_get_property(struct power_supply *psy,
 		return -EINVAL;
 	}
 
-	/* Convert units to match requirements for power supply class */
-	bq20z75_unit_adjustment(client, psp, val);
+	if (!bq20z75_device->enable_detection)
+		goto done;
+
+	if (!bq20z75_device->gpio_detect &&
+		bq20z75_device->is_present != (ret >= 0)) {
+		bq20z75_device->is_present = (ret >= 0);
+		power_supply_changed(&bq20z75_device->power_supply);
+	}
+
+done:
+	if (!ret) {
+		/* Convert units to match requirements for power supply class */
+		bq20z75_unit_adjustment(client, psp, val);
+	}
 
 	dev_dbg(&client->dev,
 		"%s: property = %d, value = %d\n", __func__, psp, val->intval);
 
-	return 0;
+	return ret;
 }
 
-static int bq20z75_probe(struct i2c_client *client,
+static irqreturn_t bq20z75_irq(int irq, void *devid)
+{
+	struct power_supply *battery = devid;
+
+	power_supply_changed(battery);
+
+	return IRQ_HANDLED;
+}
+
+static int __devinit bq20z75_probe(struct i2c_client *client,
 	const struct i2c_device_id *id)
 {
 	struct bq20z75_info *bq20z75_device;
+	struct bq20z75_platform_data *pdata = client->dev.platform_data;
 	int rc;
+	int irq;
 
 	bq20z75_device = kzalloc(sizeof(struct bq20z75_info), GFP_KERNEL);
 	if (!bq20z75_device)
 		return -ENOMEM;
 
 	bq20z75_device->client = client;
+	bq20z75_device->enable_detection = false;
+	bq20z75_device->gpio_detect = false;
 	bq20z75_device->power_supply.name = "battery";
 	bq20z75_device->power_supply.type = POWER_SUPPLY_TYPE_BATTERY;
 	bq20z75_device->power_supply.properties = bq20z75_properties;
@@ -489,26 +527,86 @@ static int bq20z75_probe(struct i2c_client *client,
 		ARRAY_SIZE(bq20z75_properties);
 	bq20z75_device->power_supply.get_property = bq20z75_get_property;
 
+	if (pdata) {
+		bq20z75_device->gpio_detect =
+			gpio_is_valid(pdata->battery_detect);
+		bq20z75_device->pdata = pdata;
+	}
+
 	i2c_set_clientdata(client, bq20z75_device);
 
+	if (!bq20z75_device->gpio_detect)
+		goto skip_gpio;
+
+	rc = gpio_request(pdata->battery_detect, dev_name(&client->dev));
+	if (rc) {
+		dev_warn(&client->dev, "Failed to request gpio: %d\n", rc);
+		bq20z75_device->gpio_detect = false;
+		goto skip_gpio;
+	}
+
+	rc = gpio_direction_input(pdata->battery_detect);
+	if (rc) {
+		dev_warn(&client->dev, "Failed to get gpio as input: %d\n", rc);
+		gpio_free(pdata->battery_detect);
+		bq20z75_device->gpio_detect = false;
+		goto skip_gpio;
+	}
+
+	irq = gpio_to_irq(pdata->battery_detect);
+	if (irq <= 0) {
+		dev_warn(&client->dev, "Failed to get gpio as irq: %d\n", irq);
+		gpio_free(pdata->battery_detect);
+		bq20z75_device->gpio_detect = false;
+		goto skip_gpio;
+	}
+
+	rc = request_irq(irq, bq20z75_irq,
+		IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+		dev_name(&client->dev), &bq20z75_device->power_supply);
+	if (rc) {
+		dev_warn(&client->dev, "Failed to request irq: %d\n", rc);
+		gpio_free(pdata->battery_detect);
+		bq20z75_device->gpio_detect = false;
+		goto skip_gpio;
+	}
+
+	bq20z75_device->irq = irq;
+
+skip_gpio:
+
 	rc = power_supply_register(&client->dev, &bq20z75_device->power_supply);
 	if (rc) {
 		dev_err(&client->dev,
 			"%s: Failed to register power supply\n", __func__);
-		kfree(bq20z75_device);
-		return rc;
+		goto exit_psupply;
 	}
 
 	dev_info(&client->dev,
 		"%s: battery gas gauge device registered\n", client->name);
 
 	return 0;
+
+exit_psupply:
+	if (bq20z75_device->irq)
+		free_irq(bq20z75_device->irq, &bq20z75_device->power_supply);
+	if (bq20z75_device->gpio_detect)
+		gpio_free(pdata->battery_detect);
+
+	kfree(bq20z75_device);
+
+	return rc;
 }
 
-static int bq20z75_remove(struct i2c_client *client)
+static int __devexit bq20z75_remove(struct i2c_client *client)
 {
 	struct bq20z75_info *bq20z75_device = i2c_get_clientdata(client);
 
+	if (bq20z75_device->irq)
+		free_irq(bq20z75_device->irq, &bq20z75_device->power_supply);
+	if (bq20z75_device->gpio_detect)
+		gpio_free(bq20z75_device->pdata->battery_detect);
+
 	power_supply_unregister(&bq20z75_device->power_supply);
 	kfree(bq20z75_device);
 	bq20z75_device = NULL;
@@ -544,7 +642,7 @@ static const struct i2c_device_id bq20z75_id[] = {
 
 static struct i2c_driver bq20z75_battery_driver = {
 	.probe		= bq20z75_probe,
-	.remove		= bq20z75_remove,
+	.remove		= __devexit_p(bq20z75_remove),
 	.suspend	= bq20z75_suspend,
 	.resume		= bq20z75_resume,
 	.id_table	= bq20z75_id,
diff --git a/include/linux/power/bq20z75.h b/include/linux/power/bq20z75.h
new file mode 100644
index 0000000..0e1b8a2
--- /dev/null
+++ b/include/linux/power/bq20z75.h
@@ -0,0 +1,37 @@
+/*
+ * Gas Gauge driver for TI's BQ20Z75
+ *
+ * Copyright (c) 2010, NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+#ifndef __LINUX_POWER_BQ20Z75_H_
+#define __LINUX_POWER_BQ20Z75_H_
+
+#include <linux/power_supply.h>
+#include <linux/types.h>
+
+/**
+ * struct bq20z75_platform_data - platform data for bq20z75 devices
+ * @battery_detect:		GPIO which is used to detect battery presence
+ * @battery_detect_present:	gpio state when battery is present (0 / 1)
+ */
+struct bq20z75_platform_data {
+	int battery_detect;
+	int battery_detect_present;
+};
+
+#endif
-- 
1.7.0.4

[PATCH 2/4] power: bq20z75: add i2c retry mechanism

[rklein]

From: Rhyland Klein <rklein@nvidia.com>

With the support of platform data, now adding support for option i2c retries
on read/write failures. Ths is specified through the optional platform data.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/power/bq20z75.c       |   37 +++++++++++++++++++++++++++++++------
 include/linux/power/bq20z75.h |    2 ++
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/power/bq20z75.c b/drivers/power/bq20z75.c
index a51e98d..e82d10e 100644
--- a/drivers/power/bq20z75.c
+++ b/drivers/power/bq20z75.c
@@ -156,30 +156,55 @@ struct bq20z75_info {
 
 static int bq20z75_read_word_data(struct i2c_client *client, u8 address)
 {
-	s32 ret;
+	struct bq20z75_info *bq20z75_device = i2c_get_clientdata(client);
+	s32 ret = 0;
+	int retries = 1;
+
+	if (bq20z75_device->pdata)
+		retries = max(bq20z75_device->pdata->i2c_retry_count + 1, 1);
+
+	while (retries > 0) {
+		ret = i2c_smbus_read_word_data(client, address);
+		if (ret >= 0)
+			break;
+		retries--;
+	}
 
-	ret = i2c_smbus_read_word_data(client, address);
 	if (ret < 0) {
-		dev_err(&client->dev,
+		dev_warn(&client->dev,
 			"%s: i2c read at address 0x%x failed\n",
 			__func__, address);
 		return ret;
 	}
+
 	return le16_to_cpu(ret);
 }
 
 static int bq20z75_write_word_data(struct i2c_client *client, u8 address,
 	u16 value)
 {
-	s32 ret;
+	struct bq20z75_info *bq20z75_device = i2c_get_clientdata(client);
+	s32 ret = 0;
+	int retries = 1;
+
+	if (bq20z75_device->pdata)
+		retries = max(bq20z75_device->pdata->i2c_retry_count + 1, 1);
+
+	while (retries > 0) {
+		ret = i2c_smbus_write_word_data(client, address,
+			le16_to_cpu(value));
+		if (ret >= 0)
+			break;
+		retries--;
+	}
 
-	ret = i2c_smbus_write_word_data(client, address, le16_to_cpu(value));
 	if (ret < 0) {
-		dev_err(&client->dev,
+		dev_warn(&client->dev,
 			"%s: i2c write to address 0x%x failed\n",
 			__func__, address);
 		return ret;
 	}
+
 	return 0;
 }
 
diff --git a/include/linux/power/bq20z75.h b/include/linux/power/bq20z75.h
index 0e1b8a2..b0843b6 100644
--- a/include/linux/power/bq20z75.h
+++ b/include/linux/power/bq20z75.h
@@ -28,10 +28,12 @@
  * struct bq20z75_platform_data - platform data for bq20z75 devices
  * @battery_detect:		GPIO which is used to detect battery presence
  * @battery_detect_present:	gpio state when battery is present (0 / 1)
+ * @i2c_retry_count:		# of times to retry on i2c IO failure
  */
 struct bq20z75_platform_data {
 	int battery_detect;
 	int battery_detect_present;
+	int i2c_retry_count;
 };
 
 #endif
-- 
1.7.0.4

[PATCH 3/4] power: bq20z75: add support for POWER_NOW

[rklein]

From: Rhyland Klein <rklein@nvidia.com>

Adding support for POWER_NOW property. Returns the product of current and
voltage in terms of uW.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/power/bq20z75.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/drivers/power/bq20z75.c b/drivers/power/bq20z75.c
index e82d10e..7558d51 100644
--- a/drivers/power/bq20z75.c
+++ b/drivers/power/bq20z75.c
@@ -142,6 +142,7 @@ static enum power_supply_property bq20z75_properties[] = {
 	POWER_SUPPLY_PROP_CHARGE_NOW,
 	POWER_SUPPLY_PROP_CHARGE_FULL,
 	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
+	POWER_SUPPLY_PROP_POWER_NOW,
 };
 
 struct bq20z75_info {
@@ -411,6 +412,32 @@ static int bq20z75_get_battery_capacity(struct i2c_client *client,
 	return 0;
 }
 
+static int bq20z75_get_power_now(struct i2c_client *client,
+	union power_supply_propval *val)
+{
+	int voltage_now = 0;
+	int current_now = 0;
+
+	voltage_now = bq20z75_read_word_data(client,
+		bq20z75_data[REG_VOLTAGE].addr);
+	if (voltage_now < 0)
+		return voltage_now;
+
+	current_now = bq20z75_read_word_data(client,
+		bq20z75_data[REG_CURRENT].addr);
+	if (current_now < 0)
+		return current_now;
+
+	/* returned values are 16 bit */
+	current_now = (s16)current_now;
+	/* need to ensure it is positive */
+	current_now = abs(current_now);
+
+	val->intval = voltage_now * current_now;
+
+	return 0;
+}
+
 static char bq20z75_serial[5];
 static int bq20z75_get_battery_serial_number(struct i2c_client *client,
 	union power_supply_propval *val)
@@ -475,6 +502,10 @@ static int bq20z75_get_property(struct power_supply *psy,
 		ret = bq20z75_get_battery_capacity(client, ret, psp, val);
 		break;
 
+	case POWER_SUPPLY_PROP_POWER_NOW:
+		ret = bq20z75_get_power_now(client, val);
+		break;
+
 	case POWER_SUPPLY_PROP_SERIAL_NUMBER:
 		ret = bq20z75_get_battery_serial_number(client, val);
 		break;
-- 
1.7.0.4

[PATCH 4/4] power_supply: update power_supply_is_watt_property

[rklein]

From: Rhyland Klein <rklein@nvidia.com>

Update the power_supply_is_watt_property function to include POWER_NOW.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 include/linux/power_supply.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 20f23fe..204c18d 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -251,6 +251,7 @@ static inline bool power_supply_is_watt_property(enum power_supply_property psp)
 	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
 	case POWER_SUPPLY_PROP_VOLTAGE_AVG:
+	case POWER_SUPPLY_PROP_POWER_NOW:
 		return 1;
 	default:
 		break;
-- 
1.7.0.4

Re: [PATCH 3/4] power: bq20z75: add support for POWER_NOW

[Anton Vorontsov]

On Mon, Feb 28, 2011 at 04:55:30PM -0800, rklein@nvidia.com wrote:
[...]
> +static int bq20z75_get_power_now(struct i2c_client *client,
> +	union power_supply_propval *val)
> +{
> +	int voltage_now = 0;
> +	int current_now = 0;
> +
> +	voltage_now = bq20z75_read_word_data(client,
> +		bq20z75_data[REG_VOLTAGE].addr);
> +	if (voltage_now < 0)
> +		return voltage_now;
> +
> +	current_now = bq20z75_read_word_data(client,
> +		bq20z75_data[REG_CURRENT].addr);
> +	if (current_now < 0)
> +		return current_now;
> +
> +	/* returned values are 16 bit */
> +	current_now = (s16)current_now;
> +	/* need to ensure it is positive */
> +	current_now = abs(current_now);
> +
> +	val->intval = voltage_now * current_now;

Nope, we don't do this in the drivers. It's a derived property, and
driver should not bother with these.

Documentation/power/power_supply_class.txt says:
- - - -
Q: Suppose, my battery monitoring chip/firmware does not provides capacity
   in percents, but provides charge_{now,full,empty}. Should I calculate
   percentage capacity manually, inside the driver, and register CAPACITY
   attribute? The same question about time_to_empty/time_to_full.
A: Most likely, no. This class is designed to export properties which are
   directly measurable by the specific hardware available.

   Inferring not available properties using some heuristics or mathematical
   model is not subject of work for a battery driver. Such functionality
   should be factored out, and in fact, apm_power, the driver to serve
   legacy APM API on top of power supply class, uses a simple heuristic of
   approximating remaining battery capacity based on its charge, current,
   voltage and so on. But full-fledged battery model is likely not subject
   for kernel at all, as it would require floating point calculation to deal
   with things like differential equations and Kalman filters. This is
   better be handled by batteryd/libbattery, yet to be written.
- - - -

There were some ideas to write a kernel module that would provide
such "derived" properties for all the drivers. But drivers themselves
should not duplicate the generic logic.

Thanks,

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

Re: [PATCH 0/4] power: bq20z75: update support

[Anton Vorontsov]

On Mon, Feb 28, 2011 at 04:55:27PM -0800, rklein@nvidia.com wrote:
> From: Rhyland Klein <rklein@nvidia.com>
> 
> Adds support to driver for the following:
> * optional gpio for battery detection
> * optional retries for i2c I/O failures
> * POWER_NOW property
> 
> For more detailed information, see specifics patches.
> 
> Rhyland Klein (4):
>   power: bq20z75: add optional battery detect gpio
>   power: bq20z75: add i2c retry mechanism
>   power: bq20z75: add support for POWER_NOW
>   power_supply: update power_supply_is_watt_property

You're fast! :-)

Applied all the patches, modulo "add support for POWER_NOW".

Thanks!

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com