2011-02-24 23:48:36

by Rhyland Klein

[permalink] [raw]
Subject: [PATCH v2] power: bq20z75: change to platform driver

From: Rhyland Klein <[email protected]>

This change switches the bq20z75 i2c driver into a platform driver
and adds support for an optional battery detect gpio. Now the driver also
signals when the battery is inserted or removed.

I also added a retry mechanism so that for boards where there may be
intermittent i2c issues, retries can be used to avoid spurious failures.

Signed-off-by: Rhyland Klein <[email protected]>
---

Changes for v2:
- changed I2C_BOARD_INFO to devinitdata to avoid Section Mismatch warning

drivers/power/bq20z75.c | 282 ++++++++++++++++++++++++++++++++---------
include/linux/power/bq20z75.h | 39 ++++++
2 files changed, 258 insertions(+), 63 deletions(-)
create mode 100644 include/linux/power/bq20z75.h

diff --git a/drivers/power/bq20z75.c b/drivers/power/bq20z75.c
index 4141775..14a65a7 100644
--- a/drivers/power/bq20z75.c
+++ b/drivers/power/bq20z75.c
@@ -25,6 +25,11 @@
#include <linux/power_supply.h>
#include <linux/i2c.h>
#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/gpio.h>
+
+#include <linux/power/bq20z75.h>

enum {
REG_MANUFACTURER_DATA,
@@ -141,17 +146,30 @@ static enum power_supply_property bq20z75_properties[] = {
};

struct bq20z75_info {
- struct i2c_client *client;
- struct power_supply power_supply;
+ struct i2c_client *client;
+ struct i2c_adapter *adapter;
+ struct power_supply power_supply;
+ bool is_present;
+ struct bq20z75_platform_data *pdata;
+ struct platform_device *pdev;
+ bool gpio_detect;
+ int irq;
+ bool enable_detection;
};

static int bq20z75_read_word_data(struct i2c_client *client, u8 address)
{
- s32 ret;
+ s32 ret = 0;
+ struct bq20z75_info *bq20z75_device = i2c_get_clientdata(client);
+ int retries = max(bq20z75_device->pdata->i2c_retry_count+1, 1);

- ret = i2c_smbus_read_word_data(client, address);
+ while (retries > 0) {
+ ret = i2c_smbus_read_word_data(client, address);
+ if (ret >= 0)
+ break;
+ }
if (ret < 0) {
- dev_err(&client->dev,
+ dev_warn(&bq20z75_device->pdev->dev,
"%s: i2c read at address 0x%x failed\n",
__func__, address);
return ret;
@@ -162,15 +180,24 @@ static int bq20z75_read_word_data(struct i2c_client *client, u8 address)
static int bq20z75_write_word_data(struct i2c_client *client, u8 address,
u16 value)
{
- s32 ret;
-
- ret = i2c_smbus_write_word_data(client, address, le16_to_cpu(value));
+ s32 ret = 0;
+ struct bq20z75_info *bq20z75_device = i2c_get_clientdata(client);
+ int 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--;
+ }
if (ret < 0) {
- dev_err(&client->dev,
+ dev_warn(&bq20z75_device->pdev->dev,
"%s: i2c write to address 0x%x failed\n",
__func__, address);
return ret;
}
+
return 0;
}

@@ -179,7 +206,15 @@ 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);
+ val->intval = (ret == 0 ? 1 : 0);
+ return ret;
+ }
/* Write to ManufacturerAccess with
* ManufacturerAccess command and then
* read the status */
@@ -189,12 +224,15 @@ static int bq20z75_get_battery_presence_and_health(
if (ret < 0)
return ret;

-
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) {
val->intval = 0;
@@ -271,6 +309,8 @@ static void bq20z75_unit_adjustment(struct i2c_client *client,
#define BATTERY_MODE_CAP_MULT_WATT (10 * BASE_UNIT_CONVERSION)
#define TIME_UNIT_CONVERSION 600
#define TEMP_KELVIN_TO_CELCIUS 2731
+ struct bq20z75_info *bq20z75_device = i2c_get_clientdata(client);
+
switch (psp) {
case POWER_SUPPLY_PROP_ENERGY_NOW:
case POWER_SUPPLY_PROP_ENERGY_FULL:
@@ -300,7 +340,7 @@ static void bq20z75_unit_adjustment(struct i2c_client *client,
break;

default:
- dev_dbg(&client->dev,
+ dev_dbg(&bq20z75_device->pdev->dev,
"%s: no need for unit conversion %d\n", __func__, psp);
}
}
@@ -348,11 +388,11 @@ static int bq20z75_get_battery_capacity(struct i2c_client *client,
if (ret < 0)
return ret;

- if (psp == POWER_SUPPLY_PROP_CAPACITY) {
- /* bq20z75 spec says that this can be >100 %
- * even if max value is 100 % */
+ /* bq20z75 spec says that this can be >100 %
+ * even if max value is 100 % */
+ if (psp == POWER_SUPPLY_PROP_CAPACITY)
val->intval = min(ret, 100);
- } else
+ else
val->intval = ret;

ret = bq20z75_set_battery_mode(client, mode);
@@ -383,11 +423,13 @@ static int bq20z75_get_property_index(struct i2c_client *client,
enum power_supply_property psp)
{
int count;
+ struct bq20z75_info *bq20z75_device = i2c_get_clientdata(client);
+
for (count = 0; count < ARRAY_SIZE(bq20z75_data); count++)
if (psp == bq20z75_data[count].psp)
return count;

- dev_warn(&client->dev,
+ dev_warn(&bq20z75_device->pdev->dev,
"%s: Invalid Property - %d\n", __func__, psp);

return -EINVAL;
@@ -398,7 +440,7 @@ static int bq20z75_get_property(struct power_supply *psy,
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 +449,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:
@@ -423,19 +463,16 @@ static int bq20z75_get_property(struct power_supply *psy,
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;
+ if (ps_index < 0) {
+ ret = ps_index;
+ break;
+ }

ret = bq20z75_get_battery_capacity(client, ps_index, psp, val);
- if (ret)
- return ret;
-
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:
@@ -447,41 +484,76 @@ static int bq20z75_get_property(struct power_supply *psy,
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;
+ if (ps_index < 0) {
+ ret = ps_index;
+ break;
+ }

ret = bq20z75_get_battery_property(client, ps_index, psp, val);
- if (ret)
- return ret;
-
break;

default:
- dev_err(&client->dev,
+ dev_err(&bq20z75_device->pdev->dev,
"%s: INVALID property\n", __func__);
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);
+ }

- dev_dbg(&client->dev,
+done:
+ if (!ret) {
+ /* Convert units to match requirements for power supply class */
+ bq20z75_unit_adjustment(client, psp, val);
+ }
+
+ dev_dbg(&bq20z75_device->pdev->dev,
"%s: property = %d, value = %d\n", __func__, psp, val->intval);

- return 0;
+ return ret;
}

-static int bq20z75_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static irqreturn_t bq20z75_irq(int irq, void *devid)
+{
+ struct power_supply *battery = devid;
+
+ power_supply_changed(battery);
+
+ return IRQ_HANDLED;
+}
+
+static struct i2c_board_info __devinitdata bq20z75_i2c_device = {
+ I2C_BOARD_INFO("bq20z75", 0x0b),
+};
+
+static int __devinit bq20z75_probe(struct platform_device *pdev)
{
struct bq20z75_info *bq20z75_device;
int rc;
+ struct bq20z75_platform_data *pdata = pdev->dev.platform_data;
+ int irq;
+ struct i2c_adapter *adapter;
+ struct i2c_client *client;
+
+ if (!pdata) {
+ dev_err(&pdev->dev, "No platform data\n");
+ return -EINVAL;
+ }

bq20z75_device = kzalloc(sizeof(struct bq20z75_info), GFP_KERNEL);
if (!bq20z75_device)
return -ENOMEM;

- bq20z75_device->client = client;
+ bq20z75_device->gpio_detect = gpio_is_valid(pdata->battery_detect);
+ bq20z75_device->pdata = pdata;
+ bq20z75_device->enable_detection = false;
+ bq20z75_device->pdev = pdev;
bq20z75_device->power_supply.name = "battery";
bq20z75_device->power_supply.type = POWER_SUPPLY_TYPE_BATTERY;
bq20z75_device->power_supply.properties = bq20z75_properties;
@@ -489,27 +561,116 @@ static int bq20z75_probe(struct i2c_client *client,
ARRAY_SIZE(bq20z75_properties);
bq20z75_device->power_supply.get_property = bq20z75_get_property;

+ platform_set_drvdata(pdev, &bq20z75_device);
+
+ if (!bq20z75_device->gpio_detect)
+ goto skip_gpio;
+
+ rc = gpio_request(pdata->battery_detect, dev_name(&pdev->dev));
+ if (rc) {
+ dev_warn(&pdev->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(&pdev->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) {
+ 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(&pdev->dev), &bq20z75_device->power_supply);
+ if (rc) {
+ dev_warn(&pdev->dev, "Failed to request irq: %d\n", rc);
+ gpio_free(pdata->battery_detect);
+ bq20z75_device->gpio_detect = false;
+ }
+
+ bq20z75_device->irq = irq;
+
+skip_gpio:
+ adapter = i2c_get_adapter(pdata->bus);
+ if (!adapter) {
+ dev_err(&pdev->dev, "Failed to get I2C adapter\n");
+ rc = -EINVAL;
+ goto exit_adapter;
+ }
+
+ client = i2c_new_device(adapter, &bq20z75_i2c_device);
+ if (!client) {
+ dev_err(&pdev->dev, "Failed to create I2C device\n");
+ rc = -EINVAL;
+ goto exit_device;
+ }
+
+ bq20z75_device->adapter = adapter;
+ bq20z75_device->client = client;
i2c_set_clientdata(client, bq20z75_device);

- rc = power_supply_register(&client->dev, &bq20z75_device->power_supply);
+ rc = power_supply_register(&pdev->dev, &bq20z75_device->power_supply);
if (rc) {
- dev_err(&client->dev,
+ dev_err(&pdev->dev,
"%s: Failed to register power supply\n", __func__);
- kfree(bq20z75_device);
- return rc;
+ goto exit_psupply;
}

- dev_info(&client->dev,
+ /* NOW check if initially present */
+ if (bq20z75_device->gpio_detect) {
+ int present = gpio_get_value(pdata->battery_detect);
+ bq20z75_device->is_present = (present == 0);
+ } else {
+ union power_supply_propval val;
+ bq20z75_get_battery_presence_and_health(client,
+ POWER_SUPPLY_PROP_PRESENT, &val);
+ bq20z75_device->is_present = (val.intval != 0);
+ }
+ bq20z75_device->enable_detection = true;
+
+ dev_info(&pdev->dev,
"%s: battery gas gauge device registered\n", client->name);

return 0;
+exit_psupply:
+ i2c_unregister_device(client);
+exit_device:
+ i2c_put_adapter(adapter);
+exit_adapter:
+ 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 platform_device *pdev)
{
- struct bq20z75_info *bq20z75_device = i2c_get_clientdata(client);
+ struct bq20z75_info *bq20z75_device = platform_get_drvdata(pdev);

power_supply_unregister(&bq20z75_device->power_supply);
+
+ 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);
+
+ i2c_unregister_device(bq20z75_device->client);
+ i2c_put_adapter(bq20z75_device->adapter);
+
+ platform_set_drvdata(pdev, NULL);
kfree(bq20z75_device);
bq20z75_device = NULL;

@@ -517,13 +678,14 @@ static int bq20z75_remove(struct i2c_client *client)
}

#if defined CONFIG_PM
-static int bq20z75_suspend(struct i2c_client *client,
+static int bq20z75_suspend(struct platform_device *pdev,
pm_message_t state)
{
+ struct bq20z75_info *bq20z75_device = platform_get_drvdata(pdev);
s32 ret;

/* write to manufacturer access with sleep command */
- ret = bq20z75_write_word_data(client,
+ ret = bq20z75_write_word_data(bq20z75_device->client,
bq20z75_data[REG_MANUFACTURER_DATA].addr,
MANUFACTURER_ACCESS_SLEEP);
if (ret < 0)
@@ -537,33 +699,27 @@ static int bq20z75_suspend(struct i2c_client *client,
/* 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 = {
+static struct platform_driver bq20z75_driver = {
.probe = bq20z75_probe,
- .remove = bq20z75_remove,
+ .remove = __devexit_p(bq20z75_remove),
.suspend = bq20z75_suspend,
.resume = bq20z75_resume,
- .id_table = bq20z75_id,
.driver = {
.name = "bq20z75-battery",
},
};

-static int __init bq20z75_battery_init(void)
+static int __init bq20z75_init(void)
{
- return i2c_add_driver(&bq20z75_battery_driver);
+ return platform_driver_register(&bq20z75_driver);
}
-module_init(bq20z75_battery_init);
+module_init(bq20z75_init);

-static void __exit bq20z75_battery_exit(void)
+static void __exit bq20z75_exit(void)
{
- i2c_del_driver(&bq20z75_battery_driver);
+ platform_driver_unregister(&bq20z75_driver);
}
-module_exit(bq20z75_battery_exit);
+module_exit(bq20z75_exit);

MODULE_DESCRIPTION("BQ20z75 battery monitor driver");
MODULE_LICENSE("GPL");
diff --git a/include/linux/power/bq20z75.h b/include/linux/power/bq20z75.h
new file mode 100644
index 0000000..53529f0
--- /dev/null
+++ b/include/linux/power/bq20z75.h
@@ -0,0 +1,39 @@
+/*
+ * 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
+ * @i2c_retry_count: # of times to retry on i2c io failure
+ * @bus: i2c bus number
+ */
+struct bq20z75_platform_data {
+ int battery_detect;
+ int i2c_retry_count;
+ int bus;
+};
+
+#endif
--
1.7.0.4


2011-02-28 14:04:53

by Anton Vorontsov

[permalink] [raw]
Subject: Re: [PATCH v2] power: bq20z75: change to platform driver

On Thu, Feb 24, 2011 at 04:49:37PM -0800, Rhyland Klein wrote:
> On Thu, 2011-02-24 at 15:48 -0800, Rhyland Klein wrote:
> > From: Rhyland Klein <[email protected]>
> >
> > This change switches the bq20z75 i2c driver into a platform driver
> > and adds support for an optional battery detect gpio. Now the driver also
> > signals when the battery is inserted or removed.
> >
> > I also added a retry mechanism so that for boards where there may be
> > intermittent i2c issues, retries can be used to avoid spurious failures.
> >
> > Signed-off-by: Rhyland Klein <[email protected]>
> > ---
[...]
> Anton, are you going to have a chance to review this? I have one more
> patch to submit but I was waiting for this to go in first.

Sorry for the delay.

The main problem with this patch is that it does too many things.
It should be separated into three patches:

1. Convert the driver into a platform driver;
2. Add support for detect gpio;
3. Add retry machanism.

But still, why are you converting the i2c driver into a platform driver?
To pass the GPIOs via platform data? You can do this with I2C driver as
well:

struct i2c_board_info {
....
void *platform_data;

Thanks,

--
Anton Vorontsov
Email: [email protected]