linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC
@ 2010-09-02 17:29 rklein-DDmLM1+adcrQT0dZR+AlfA
       [not found] ` <1283448586-14613-1-git-send-email-rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2010-09-03  9:43 ` Mark Brown
  0 siblings, 2 replies; 8+ messages in thread
From: rklein-DDmLM1+adcrQT0dZR+AlfA @ 2010-09-02 17:29 UTC (permalink / raw)
  To: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, olof-nZhT3qVonbNeoWH0uzbU5w,
	achew-DDmLM1+adcrQT0dZR+AlfA, khali-PUYAD+kWke1g9hUCZPvPmw,
	broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E,
	Rhyland Klein

From: Rhyland Klein <rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

this driver depends on I2C and uses SMBUS for communication with the host.

Addressed comments from reviews by Mark Brown and Jean Delvare.
  * Cleaned up whitespace and alignment issues
  * changed return codes to more appropriate values
  * change Kconfig option name to be consistent with existing devices
  * removed global struct and moved to device specific data
  * changed printk to dev_dbg

Signed-off-by: Rhyland Klein <rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/power/Kconfig   |    7 +
 drivers/power/Makefile  |    1 +
 drivers/power/bq20z75.c |  387 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 395 insertions(+), 0 deletions(-)
 create mode 100644 drivers/power/bq20z75.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 8e9ba17..53d9cbf 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -142,4 +142,11 @@ config CHARGER_PCF50633
 	help
 	 Say Y to include support for NXP PCF50633 Main Battery Charger.
 
+config BATTERY_BQ20Z75
+        tristate "TI BQ20z75 gas gauge"
+        depends on I2C
+        help
+         Say Y to include support for TI bq20z75 SBS-compliant
+         gas gauge and protection IC.
+
 endif # POWER_SUPPLY
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 0005080..cdc403d 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -34,3 +34,4 @@ obj-$(CONFIG_BATTERY_DA9030)	+= da9030_battery.o
 obj-$(CONFIG_BATTERY_MAX17040)	+= max17040_battery.o
 obj-$(CONFIG_BATTERY_Z2)	+= z2_battery.o
 obj-$(CONFIG_CHARGER_PCF50633)	+= pcf50633-charger.o
+obj-$(CONFIG_BATTERY_BQ20Z75)	+= bq20z75.o
diff --git a/drivers/power/bq20z75.c b/drivers/power/bq20z75.c
new file mode 100644
index 0000000..a7eeedf
--- /dev/null
+++ b/drivers/power/bq20z75.c
@@ -0,0 +1,387 @@
+/*
+ * 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.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/power_supply.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+
+enum {
+	REG_MANUFACTURER_DATA,
+	REG_TEMPERATURE,
+	REG_VOLTAGE,
+	REG_CURRENT,
+	REG_CAPACITY,
+	REG_TIME_TO_EMPTY,
+	REG_TIME_TO_FULL,
+	REG_STATUS,
+	REG_CYCLE_COUNT,
+	REG_SERIAL_NUMBER
+};
+
+/* manufacturer access defines */
+#define MANUFACTURER_ACCESS_STATUS	0x0006
+#define MANUFACTURER_ACCESS_SLEEP	0x0011
+
+/* battery status value bits */
+#define BATTERY_CHARGING		0x40
+#define BATTERY_FULL_CHARGED		0x20
+#define BATTERY_FULL_DISCHARGED		0x10
+
+#define BQ20Z75_DATA(_psp, _addr, _min_value, _max_value) { \
+	.psp = _psp, \
+	.addr = _addr, \
+	.min_value = _min_value, \
+	.max_value = _max_value, \
+}
+
+static const struct bq20z75_device_data {
+	enum power_supply_property psp;
+	u8 addr;
+	int min_value;
+	int max_value;
+} bq20z75_data[] = {
+	[REG_MANUFACTURER_DATA] =
+		BQ20Z75_DATA(POWER_SUPPLY_PROP_PRESENT, 0x00, 0, 65535),
+	[REG_TEMPERATURE] =
+		BQ20Z75_DATA(POWER_SUPPLY_PROP_TEMP, 0x08, 0, 65535),
+	[REG_VOLTAGE] =
+		BQ20Z75_DATA(POWER_SUPPLY_PROP_VOLTAGE_NOW, 0x09, 0, 20000),
+	[REG_CURRENT] =
+		BQ20Z75_DATA(POWER_SUPPLY_PROP_CURRENT_NOW, 0x0A, -32768,
+			32767),
+	[REG_CAPACITY] =
+		BQ20Z75_DATA(POWER_SUPPLY_PROP_CAPACITY, 0x0E, 0, 100),
+	[REG_TIME_TO_EMPTY] =
+		BQ20Z75_DATA(POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG, 0x12, 0,
+			65535),
+	[REG_TIME_TO_FULL] =
+		BQ20Z75_DATA(POWER_SUPPLY_PROP_TIME_TO_FULL_AVG, 0x13, 0,
+			65535),
+	[REG_STATUS] =
+		BQ20Z75_DATA(POWER_SUPPLY_PROP_STATUS, 0x16, 0, 65535),
+	[REG_CYCLE_COUNT] =
+		BQ20Z75_DATA(POWER_SUPPLY_PROP_CYCLE_COUNT, 0x17, 0, 65535),
+	[REG_SERIAL_NUMBER] =
+		BQ20Z75_DATA(POWER_SUPPLY_PROP_SERIAL_NUMBER, 0x1C, 0, 65535),
+};
+
+static enum power_supply_property bq20z75_properties[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_CYCLE_COUNT,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
+	POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
+	POWER_SUPPLY_PROP_SERIAL_NUMBER,
+};
+
+static int bq20z75_get_property(struct power_supply *psy,
+	enum power_supply_property psp, union power_supply_propval *val);
+
+struct bq20z75_info {
+	struct i2c_client	*client;
+	struct power_supply	power_supply;
+};
+
+static int bq20z75_get_battery_presence_and_health(
+	struct i2c_client *client, enum power_supply_property psp,
+	union power_supply_propval *val)
+{
+	s32 ret;
+
+	/* Write to ManufacturerAccess with
+	 * ManufacturerAccess command and then
+	 * read the status */
+	ret = i2c_smbus_write_word_data(client,
+		bq20z75_data[REG_MANUFACTURER_DATA].addr,
+		MANUFACTURER_ACCESS_STATUS);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"%s: i2c write for battery presence failed\n",
+			__func__);
+		return -ENODEV;
+	}
+
+	ret = i2c_smbus_read_word_data(client,
+		bq20z75_data[REG_MANUFACTURER_DATA].addr);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"%s: i2c read for battery presence failed\n",
+			__func__);
+		return -EIO;
+	}
+
+	if (ret < bq20z75_data[REG_MANUFACTURER_DATA].min_value ||
+	    ret > bq20z75_data[REG_MANUFACTURER_DATA].max_value) {
+		val->intval = 0;
+		return 0;
+	}
+
+	/* Mask the upper nibble of 2nd byte and
+	 * lower byte of response then
+	 * shift the result by 8 to get status*/
+	ret &= 0x0F00;
+	ret >>= 8;
+	if (psp == POWER_SUPPLY_PROP_PRESENT) {
+		if (ret == 0x0F)
+			/* battery removed */
+			val->intval = 0;
+		else
+			val->intval = 1;
+	} else if (psp == POWER_SUPPLY_PROP_HEALTH) {
+		if (ret == 0x09)
+			val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
+		else if (ret == 0x0B)
+			val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+		else if (ret == 0x0C)
+			val->intval = POWER_SUPPLY_HEALTH_DEAD;
+		else
+			val->intval = POWER_SUPPLY_HEALTH_GOOD;
+	}
+
+	return 0;
+}
+
+static int bq20z75_get_battery_property(struct i2c_client *client, 
+	int reg_offset, enum power_supply_property psp, 
+	union power_supply_propval *val)
+{
+	s32 ret;
+
+	ret = i2c_smbus_read_word_data(client,
+		bq20z75_data[reg_offset].addr);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"%s: i2c read for %d failed\n", __func__, reg_offset);
+		return -EIO;
+	}
+
+	if (ret >= bq20z75_data[reg_offset].min_value &&
+	    ret <= bq20z75_data[reg_offset].max_value) {
+		val->intval = ret;
+		if (psp == POWER_SUPPLY_PROP_STATUS) {
+			if (ret & BATTERY_CHARGING)
+				val->intval = POWER_SUPPLY_STATUS_CHARGING;
+			else if (ret & BATTERY_FULL_CHARGED)
+				val->intval = POWER_SUPPLY_STATUS_FULL;
+			else if (ret & BATTERY_FULL_DISCHARGED)
+				val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+			else
+				val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+		}
+		/* bq20z75 provides battery tempreture in 0.1°K
+		 * so convert it to °C */
+		else if (psp == POWER_SUPPLY_PROP_TEMP)
+			val->intval = ret - 2731;
+	} else {
+		if (psp == POWER_SUPPLY_PROP_STATUS)
+			val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+		else
+			val->intval = 0;
+	}
+
+	return 0;
+}
+
+static int bq20z75_get_battery_capacity(struct i2c_client *client,
+	union power_supply_propval *val)
+{
+	s32 ret;
+
+	ret = i2c_smbus_read_byte_data(client, bq20z75_data[REG_CAPACITY].addr);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"%s: i2c read for %d failed\n", __func__, REG_CAPACITY);
+		return -EIO;
+	}
+
+	/* bq20z75 spec says that this can be >100 %
+	 * even if max value is 100 % */
+	val->intval = min(ret, 100);
+
+	return 0;
+}
+
+static int bq20z75_get_property(struct power_supply *psy,
+	enum power_supply_property psp,
+	union power_supply_propval *val)
+{
+	int count;
+	int ret;
+	struct bq20z75_info *bq20z75_device = container_of(psy,
+				struct bq20z75_info, power_supply);
+	struct i2c_client *client = bq20z75_device->client;
+
+	switch (psp) {
+	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:
+		val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
+		break;
+
+	case POWER_SUPPLY_PROP_CAPACITY:
+		ret = bq20z75_get_battery_capacity(client, val);
+		if (ret)
+			return ret;
+		break;
+
+	case POWER_SUPPLY_PROP_STATUS:
+	case POWER_SUPPLY_PROP_CYCLE_COUNT:
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+	case POWER_SUPPLY_PROP_TEMP:
+	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
+	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
+	case POWER_SUPPLY_PROP_SERIAL_NUMBER:
+		for (count = 0; count < ARRAY_SIZE(bq20z75_data); count++) {
+			if (psp == bq20z75_data[count].psp)
+				break;
+		}
+
+		ret = bq20z75_get_battery_property(client, count, psp, val);
+		if (ret)
+			return ret;
+		break;
+
+	default:
+		dev_err(&client->dev,
+			"%s: INVALID property\n", __func__);
+		return -EINVAL;
+	}
+
+	dev_dbg(&client->dev,
+		"%s: property = %d, value = %d\n", __func__, psp, val->intval);
+
+	return 0;
+}
+
+static int bq20z75_probe(struct i2c_client *client,
+	const struct i2c_device_id *id)
+{
+	struct bq20z75_info *bq20z75_device;
+	int rc;
+
+	bq20z75_device = kzalloc(sizeof(struct bq20z75_info), GFP_KERNEL);
+	if (!bq20z75_device)
+		return -ENOMEM;
+
+	bq20z75_device->client = client;
+	bq20z75_device->power_supply.name = "battery";
+	bq20z75_device->power_supply.type = POWER_SUPPLY_TYPE_BATTERY;
+	bq20z75_device->power_supply.properties = bq20z75_properties;
+	bq20z75_device->power_supply.num_properties = ARRAY_SIZE(bq20z75_properties);
+	bq20z75_device->power_supply.get_property = bq20z75_get_property;
+
+	i2c_set_clientdata(client, bq20z75_device);
+
+	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;
+	}
+
+	dev_info(&client->dev,
+		"%s: battery gas gauge device registered\n", client->name);
+
+	return 0;
+}
+
+static int bq20z75_remove(struct i2c_client *client)
+{
+	struct bq20z75_info *bq20z75_device = i2c_get_clientdata(client);
+
+	power_supply_unregister(&bq20z75_device->power_supply);
+	kfree(bq20z75_device);
+	bq20z75_device = NULL;
+
+	return 0;
+}
+
+#if defined CONFIG_PM
+static int bq20z75_suspend(struct i2c_client *client,
+	pm_message_t state)
+{
+	s32 ret;
+
+	/* write to manufacturer access with sleep command */
+	ret = i2c_smbus_write_word_data(client,
+		bq20z75_data[REG_MANUFACTURER_DATA].addr,
+		MANUFACTURER_ACCESS_SLEEP);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"%s: i2c write for %d failed\n",
+			__func__, MANUFACTURER_ACCESS_SLEEP);
+		return -EIO;
+	}
+
+	return 0;
+}
+#else
+#define bq20z75_suspend		NULL
+#endif
+/* any smbus transaction will wake up bq20z75 */
+#define bq20z75_resume		NULL
+
+static const struct i2c_device_id bq20z75_id[] = {
+	{ "bq20z75", 0 },
+	{}
+};
+
+static struct i2c_driver bq20z75_battery_driver = {
+	.probe		= bq20z75_probe,
+	.remove		= bq20z75_remove,
+	.suspend	= bq20z75_suspend,
+	.resume		= bq20z75_resume,
+	.id_table	= bq20z75_id,
+	.driver = {
+		.name	= "bq20z75-battery",
+	},
+};
+
+static int __init bq20z75_battery_init(void)
+{
+	return i2c_add_driver(&bq20z75_battery_driver);
+}
+module_init(bq20z75_battery_init);
+
+static void __exit bq20z75_battery_exit(void)
+{
+	i2c_del_driver(&bq20z75_battery_driver);
+}
+module_exit(bq20z75_battery_exit);
+
+MODULE_DESCRIPTION("BQ20z75 battery monitor driver");
+MODULE_LICENSE("GPL");
-- 
1.7.0.4

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

* Re: [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC
       [not found] ` <1283448586-14613-1-git-send-email-rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2010-09-02 17:47   ` Anton Vorontsov
       [not found]     ` <20100902174736.GA28649-wnGakbxT3iijyJ0x5qLZdcN33GVbZNy3@public.gmane.org>
  2010-09-05 14:15   ` Jean Delvare
  1 sibling, 1 reply; 8+ messages in thread
From: Anton Vorontsov @ 2010-09-02 17:47 UTC (permalink / raw)
  To: rklein-DDmLM1+adcrQT0dZR+AlfA
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, olof-nZhT3qVonbNeoWH0uzbU5w,
	achew-DDmLM1+adcrQT0dZR+AlfA, khali-PUYAD+kWke1g9hUCZPvPmw,
	broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E

On Thu, Sep 02, 2010 at 10:29:46AM -0700, rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org wrote:
> From: Rhyland Klein <rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> this driver depends on I2C and uses SMBUS for communication with the host.
> 
> Addressed comments from reviews by Mark Brown and Jean Delvare.
>   * Cleaned up whitespace and alignment issues
>   * changed return codes to more appropriate values
>   * change Kconfig option name to be consistent with existing devices
>   * removed global struct and moved to device specific data
>   * changed printk to dev_dbg

The driver looks good to me. But I'll wait a day or two for Jean's
and Mark's Acked-by or Reviewed-by tags before applying, just to
give them some credit for reviwing this driver.

Thanks!

-- 
Anton Vorontsov
email: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
irc://irc.freenode.net/bd2

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

* Re: [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC
  2010-09-02 17:29 [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC rklein-DDmLM1+adcrQT0dZR+AlfA
       [not found] ` <1283448586-14613-1-git-send-email-rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2010-09-03  9:43 ` Mark Brown
  1 sibling, 0 replies; 8+ messages in thread
From: Mark Brown @ 2010-09-03  9:43 UTC (permalink / raw)
  To: rklein; +Cc: cbouatmailru, linux-i2c, linux-kernel, olof, achew, khali

On Thu, Sep 02, 2010 at 10:29:46AM -0700, rklein@nvidia.com wrote:
> From: Rhyland Klein <rklein@nvidia.com>
> 
> this driver depends on I2C and uses SMBUS for communication with the host.

Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

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

* RE: [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC
       [not found]     ` <20100902174736.GA28649-wnGakbxT3iijyJ0x5qLZdcN33GVbZNy3@public.gmane.org>
@ 2010-09-04 21:00       ` Rhyland Klein
  2010-09-05  8:47         ` Mark Brown
  0 siblings, 1 reply; 8+ messages in thread
From: Rhyland Klein @ 2010-09-04 21:00 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org, Andrew Chew,
	khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org,
	broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org

Mark, have you had a chance to double check my changes based on the comments you made? 

Thanks,
Rhyland

-----Original Message-----
From: Anton Vorontsov [mailto:cbouatmailru@gmail.com] 
Sent: Thursday, September 02, 2010 10:48 AM
To: Rhyland Klein
Cc: linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; olof@lixom.net; Andrew Chew; khali@linux-fr.org; broonie@opensource.wolfsonmicro.com
Subject: Re: [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC

On Thu, Sep 02, 2010 at 10:29:46AM -0700, rklein@nvidia.com wrote:
> From: Rhyland Klein <rklein@nvidia.com>
> 
> this driver depends on I2C and uses SMBUS for communication with the host.
> 
> Addressed comments from reviews by Mark Brown and Jean Delvare.
>   * Cleaned up whitespace and alignment issues
>   * changed return codes to more appropriate values
>   * change Kconfig option name to be consistent with existing devices
>   * removed global struct and moved to device specific data
>   * changed printk to dev_dbg

The driver looks good to me. But I'll wait a day or two for Jean's
and Mark's Acked-by or Reviewed-by tags before applying, just to
give them some credit for reviwing this driver.

Thanks!

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

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

* Re: [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC
  2010-09-04 21:00       ` Rhyland Klein
@ 2010-09-05  8:47         ` Mark Brown
  2010-09-05  8:49           ` Rhyland Klein
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Brown @ 2010-09-05  8:47 UTC (permalink / raw)
  To: Rhyland Klein
  Cc: Anton Vorontsov, linux-i2c@vger.kernel.org,
	linux-kernel@vger.kernel.org, olof@lixom.net, Andrew Chew,
	khali@linux-fr.org

On Sat, Sep 04, 2010 at 02:00:47PM -0700, Rhyland Klein wrote:

> Mark, have you had a chance to double check my changes based on the comments you made? 

Yes: http://lkml.org/lkml/2010/9/3/136

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

* RE: [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC
  2010-09-05  8:47         ` Mark Brown
@ 2010-09-05  8:49           ` Rhyland Klein
       [not found]             ` <E88A1D35AF5437468E5B14C7C4BA61C702CF4B6524-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Rhyland Klein @ 2010-09-05  8:49 UTC (permalink / raw)
  To: Mark Brown
  Cc: Anton Vorontsov, linux-i2c@vger.kernel.org,
	linux-kernel@vger.kernel.org, olof@lixom.net, Andrew Chew,
	khali@linux-fr.org

Thanks, sorry I somehow missed that email.

-----Original Message-----
From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com] 
Sent: Sunday, September 05, 2010 1:47 AM
To: Rhyland Klein
Cc: Anton Vorontsov; linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; olof@lixom.net; Andrew Chew; khali@linux-fr.org
Subject: Re: [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC

On Sat, Sep 04, 2010 at 02:00:47PM -0700, Rhyland Klein wrote:

> Mark, have you had a chance to double check my changes based on the comments you made? 

Yes: http://lkml.org/lkml/2010/9/3/136

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

* Re: [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC
       [not found]             ` <E88A1D35AF5437468E5B14C7C4BA61C702CF4B6524-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>
@ 2010-09-05  8:59               ` Mark Brown
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2010-09-05  8:59 UTC (permalink / raw)
  To: Rhyland Klein
  Cc: Anton Vorontsov,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org, Andrew Chew,
	khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org

On Sun, Sep 05, 2010 at 01:49:36AM -0700, Rhyland Klein wrote:

> Thanks, sorry I somehow missed that email.

No problem.

Also, please don't top post on Linux lists - it makes it much harder to
follow the thread of discussion.

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

* Re: [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC
       [not found] ` <1283448586-14613-1-git-send-email-rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2010-09-02 17:47   ` Anton Vorontsov
@ 2010-09-05 14:15   ` Jean Delvare
  1 sibling, 0 replies; 8+ messages in thread
From: Jean Delvare @ 2010-09-05 14:15 UTC (permalink / raw)
  To: Rhyland Klein
  Cc: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, olof-nZhT3qVonbNeoWH0uzbU5w,
	achew-DDmLM1+adcrQT0dZR+AlfA,
	broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E

Hi Rhyland,

On Thu,  2 Sep 2010 10:29:46 -0700, rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org wrote:
> From: Rhyland Klein <rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> this driver depends on I2C and uses SMBUS for communication with the host.
> 
> Addressed comments from reviews by Mark Brown and Jean Delvare.
>   * Cleaned up whitespace and alignment issues
>   * changed return codes to more appropriate values
>   * change Kconfig option name to be consistent with existing devices
>   * removed global struct and moved to device specific data
>   * changed printk to dev_dbg

Looks much better. A few details could be ironed out:

> 
> Signed-off-by: Rhyland Klein <rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/power/Kconfig   |    7 +
>  drivers/power/Makefile  |    1 +
>  drivers/power/bq20z75.c |  387 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 395 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/power/bq20z75.c
> 
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 8e9ba17..53d9cbf 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -142,4 +142,11 @@ config CHARGER_PCF50633
>  	help
>  	 Say Y to include support for NXP PCF50633 Main Battery Charger.
>  
> +config BATTERY_BQ20Z75
> +        tristate "TI BQ20z75 gas gauge"
> +        depends on I2C
> +        help
> +         Say Y to include support for TI bq20z75 SBS-compliant

Please leave the BQ in capitals for consistency.

> +         gas gauge and protection IC.
> +
>  endif # POWER_SUPPLY
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index 0005080..cdc403d 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -34,3 +34,4 @@ obj-$(CONFIG_BATTERY_DA9030)	+= da9030_battery.o
>  obj-$(CONFIG_BATTERY_MAX17040)	+= max17040_battery.o
>  obj-$(CONFIG_BATTERY_Z2)	+= z2_battery.o
>  obj-$(CONFIG_CHARGER_PCF50633)	+= pcf50633-charger.o
> +obj-$(CONFIG_BATTERY_BQ20Z75)	+= bq20z75.o

Would be nice to move this entry right before CONFIG_BATTERY_BQ27x00.
Same for Kconfig, BTW.

> diff --git a/drivers/power/bq20z75.c b/drivers/power/bq20z75.c
> new file mode 100644
> index 0000000..a7eeedf
> --- /dev/null
> +++ b/drivers/power/bq20z75.c
> (...)
> +static int bq20z75_get_property(struct power_supply *psy,
> +	enum power_supply_property psp, union power_supply_propval *val);

You no longer need this forward declaration.

Once these small details are fixed, feel free to add:

Reviewed-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>

-- 
Jean Delvare

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

end of thread, other threads:[~2010-09-05 14:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-02 17:29 [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC rklein-DDmLM1+adcrQT0dZR+AlfA
     [not found] ` <1283448586-14613-1-git-send-email-rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2010-09-02 17:47   ` Anton Vorontsov
     [not found]     ` <20100902174736.GA28649-wnGakbxT3iijyJ0x5qLZdcN33GVbZNy3@public.gmane.org>
2010-09-04 21:00       ` Rhyland Klein
2010-09-05  8:47         ` Mark Brown
2010-09-05  8:49           ` Rhyland Klein
     [not found]             ` <E88A1D35AF5437468E5B14C7C4BA61C702CF4B6524-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>
2010-09-05  8:59               ` Mark Brown
2010-09-05 14:15   ` Jean Delvare
2010-09-03  9:43 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).