public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] ds2782_battery: add support for ds2786 battery gas gauge
@ 2010-04-26 11:05 Mike Rapoport
  2010-04-26 18:22 ` Anton Vorontsov
  0 siblings, 1 reply; 4+ messages in thread
From: Mike Rapoport @ 2010-04-26 11:05 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: Ryan Mallon, linux-kernel, Yulia Vilensky, Mike Rapoport

From: Yulia Vilensky <vilensky@compulab.co.il>

v2 changes:
 * add ds278x_platform_data
 * address Anton comments
v3 changes:
 * use ds278x_get_temp for both ds2782 and ds2786
 * update math as per Ryan comments

Signed-off-by: Yulia Vilensky <vilensky@compulab.co.il>
Signed-off-by: Mike Rapoport <mike@compulab.co.il>
---
 drivers/power/Kconfig          |    4 +-
 drivers/power/ds2782_battery.c |  184 +++++++++++++++++++++++++++++-----------
 include/linux/ds2782_battery.h |    8 ++
 3 files changed, 146 insertions(+), 50 deletions(-)
 create mode 100644 include/linux/ds2782_battery.h

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index faaa9b4..d8b8a19 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -65,10 +65,10 @@ config BATTERY_DS2760
 	  Say Y here to enable support for batteries with ds2760 chip.
 
 config BATTERY_DS2782
-	tristate "DS2782 standalone gas-gauge"
+	tristate "DS2782/DS2786 standalone gas-gauge"
 	depends on I2C
 	help
-	  Say Y here to enable support for the DS2782 standalone battery
+	  Say Y here to enable support for the DS2782/DS2786 standalone battery
 	  gas-gauge.
 
 config BATTERY_PMU
diff --git a/drivers/power/ds2782_battery.c b/drivers/power/ds2782_battery.c
index 99c8997..2a651e9 100644
--- a/drivers/power/ds2782_battery.c
+++ b/drivers/power/ds2782_battery.c
@@ -5,6 +5,8 @@
  *
  * Author: Ryan Mallon <ryan@bluewatersys.com>
  *
+ * DS2786 added by Yulia Vilensky <vilensky@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.
@@ -20,12 +22,13 @@
 #include <linux/idr.h>
 #include <linux/power_supply.h>
 #include <linux/slab.h>
+#include <linux/ds2782_battery.h>
 
 #define DS2782_REG_RARC		0x06	/* Remaining active relative capacity */
 
-#define DS2782_REG_VOLT_MSB	0x0c
-#define DS2782_REG_TEMP_MSB	0x0a
-#define DS2782_REG_CURRENT_MSB	0x0e
+#define DS278x_REG_VOLT_MSB	0x0c
+#define DS278x_REG_TEMP_MSB	0x0a
+#define DS278x_REG_CURRENT_MSB	0x0e
 
 /* EEPROM Block */
 #define DS2782_REG_RSNSP	0x69	/* Sense resistor value */
@@ -33,18 +36,33 @@
 /* Current unit measurement in uA for a 1 milli-ohm sense resistor */
 #define DS2782_CURRENT_UNITS	1563
 
-#define to_ds2782_info(x) container_of(x, struct ds2782_info, battery)
+#define DS2786_REG_RARC		0x02	/* Remaining active relative capacity */
+
+#define DS2786_CURRENT_UNITS	25
+
+struct ds278x_info;
+
+struct ds278x_battery_ops {
+	int	(*get_current)(struct ds278x_info *info, int *current_uA);
+	int	(*get_voltage)(struct ds278x_info *info, int *voltage_uA);
+	int	(*get_capacity)(struct ds278x_info *info, int *capacity_uA);
+
+};
+
+#define to_ds278x_info(x) container_of(x, struct ds278x_info, battery)
 
-struct ds2782_info {
+struct ds278x_info {
 	struct i2c_client	*client;
 	struct power_supply	battery;
+	struct ds278x_battery_ops  *ops;
 	int			id;
+	int                     rsns;
 };
 
 static DEFINE_IDR(battery_id);
 static DEFINE_MUTEX(battery_lock);
 
-static inline int ds2782_read_reg(struct ds2782_info *info, int reg, u8 *val)
+static inline int ds278x_read_reg(struct ds278x_info *info, int reg, u8 *val)
 {
 	int ret;
 
@@ -58,7 +76,7 @@ static inline int ds2782_read_reg(struct ds2782_info *info, int reg, u8 *val)
 	return 0;
 }
 
-static inline int ds2782_read_reg16(struct ds2782_info *info, int reg_msb,
+static inline int ds278x_read_reg16(struct ds278x_info *info, int reg_msb,
 				    s16 *val)
 {
 	int ret;
@@ -73,7 +91,7 @@ static inline int ds2782_read_reg16(struct ds2782_info *info, int reg_msb,
 	return 0;
 }
 
-static int ds2782_get_temp(struct ds2782_info *info, int *temp)
+static int ds278x_get_temp(struct ds278x_info *info, int *temp)
 {
 	s16 raw;
 	int err;
@@ -84,14 +102,14 @@ static int ds2782_get_temp(struct ds2782_info *info, int *temp)
 	 * celsius. The temperature value is stored as a 10 bit number, plus
 	 * sign in the upper bits of a 16 bit register.
 	 */
-	err = ds2782_read_reg16(info, DS2782_REG_TEMP_MSB, &raw);
+	err = ds278x_read_reg16(info, DS278x_REG_TEMP_MSB, &raw);
 	if (err)
 		return err;
 	*temp = ((raw / 32) * 125) / 100;
 	return 0;
 }
 
-static int ds2782_get_current(struct ds2782_info *info, int *current_uA)
+static int ds2782_get_current(struct ds278x_info *info, int *current_uA)
 {
 	int sense_res;
 	int err;
@@ -102,7 +120,7 @@ static int ds2782_get_current(struct ds2782_info *info, int *current_uA)
 	 * The units of measurement for current are dependent on the value of
 	 * the sense resistor.
 	 */
-	err = ds2782_read_reg(info, DS2782_REG_RSNSP, &sense_res_raw);
+	err = ds278x_read_reg(info, DS2782_REG_RSNSP, &sense_res_raw);
 	if (err)
 		return err;
 	if (sense_res_raw == 0) {
@@ -113,14 +131,14 @@ static int ds2782_get_current(struct ds2782_info *info, int *current_uA)
 
 	dev_dbg(&info->client->dev, "sense resistor = %d milli-ohms\n",
 		sense_res);
-	err = ds2782_read_reg16(info, DS2782_REG_CURRENT_MSB, &raw);
+	err = ds278x_read_reg16(info, DS278x_REG_CURRENT_MSB, &raw);
 	if (err)
 		return err;
 	*current_uA = raw * (DS2782_CURRENT_UNITS / sense_res);
 	return 0;
 }
 
-static int ds2782_get_voltage(struct ds2782_info *info, int *voltage_uA)
+static int ds2782_get_voltage(struct ds278x_info *info, int *voltage_uA)
 {
 	s16 raw;
 	int err;
@@ -129,36 +147,77 @@ static int ds2782_get_voltage(struct ds2782_info *info, int *voltage_uA)
 	 * Voltage is measured in units of 4.88mV. The voltage is stored as
 	 * a 10-bit number plus sign, in the upper bits of a 16-bit register
 	 */
-	err = ds2782_read_reg16(info, DS2782_REG_VOLT_MSB, &raw);
+	err = ds278x_read_reg16(info, DS278x_REG_VOLT_MSB, &raw);
 	if (err)
 		return err;
 	*voltage_uA = (raw / 32) * 4800;
 	return 0;
 }
 
-static int ds2782_get_capacity(struct ds2782_info *info, int *capacity)
+static int ds2782_get_capacity(struct ds278x_info *info, int *capacity)
 {
 	int err;
 	u8 raw;
 
-	err = ds2782_read_reg(info, DS2782_REG_RARC, &raw);
+	err = ds278x_read_reg(info, DS2782_REG_RARC, &raw);
 	if (err)
 		return err;
 	*capacity = raw;
 	return raw;
 }
 
-static int ds2782_get_status(struct ds2782_info *info, int *status)
+static int ds2786_get_current(struct ds278x_info *info, int *current_uA)
+{
+	int err;
+	s16 raw;
+
+	err = ds278x_read_reg16(info, DS278x_REG_CURRENT_MSB, &raw);
+	if (err)
+		return err;
+	*current_uA = (raw / 16) * (DS2786_CURRENT_UNITS / info->rsns);
+	return 0;
+}
+
+static int ds2786_get_voltage(struct ds278x_info *info, int *voltage_uA)
+{
+	s16 raw;
+	int err;
+
+	/*
+	 * Voltage is measured in units of 1.22mV. The voltage is stored as
+	 * a 10-bit number plus sign, in the upper bits of a 16-bit register
+	 */
+	err = ds278x_read_reg16(info, DS278x_REG_VOLT_MSB, &raw);
+	if (err)
+		return err;
+	*voltage_uA = (raw / 8) * 1220;
+	return 0;
+}
+
+static int ds2786_get_capacity(struct ds278x_info *info, int *capacity)
+{
+	int err;
+	u8 raw;
+
+	err = ds278x_read_reg(info, DS2786_REG_RARC, &raw);
+	if (err)
+		return err;
+	/* Relative capacity is displayed with resolution 0.5 % */
+	*capacity = raw/2 ;
+	return 0;
+}
+
+static int ds278x_get_status(struct ds278x_info *info, int *status)
 {
 	int err;
 	int current_uA;
 	int capacity;
 
-	err = ds2782_get_current(info, &current_uA);
+	err = info->ops->get_current(info, &current_uA);
 	if (err)
 		return err;
 
-	err = ds2782_get_capacity(info, &capacity);
+	err = info->ops->get_capacity(info, &capacity);
 	if (err)
 		return err;
 
@@ -174,32 +233,32 @@ static int ds2782_get_status(struct ds2782_info *info, int *status)
 	return 0;
 }
 
-static int ds2782_battery_get_property(struct power_supply *psy,
+static int ds278x_battery_get_property(struct power_supply *psy,
 				       enum power_supply_property prop,
 				       union power_supply_propval *val)
 {
-	struct ds2782_info *info = to_ds2782_info(psy);
+	struct ds278x_info *info = to_ds278x_info(psy);
 	int ret;
 
 	switch (prop) {
 	case POWER_SUPPLY_PROP_STATUS:
-		ret = ds2782_get_status(info, &val->intval);
+		ret = ds278x_get_status(info, &val->intval);
 		break;
 
 	case POWER_SUPPLY_PROP_CAPACITY:
-		ret = ds2782_get_capacity(info, &val->intval);
+		ret = info->ops->get_capacity(info, &val->intval);
 		break;
 
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
-		ret = ds2782_get_voltage(info, &val->intval);
+		ret = info->ops->get_voltage(info, &val->intval);
 		break;
 
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
-		ret = ds2782_get_current(info, &val->intval);
+		ret = info->ops->get_current(info, &val->intval);
 		break;
 
 	case POWER_SUPPLY_PROP_TEMP:
-		ret = ds2782_get_temp(info, &val->intval);
+		ret = ds278x_get_temp(info, &val->intval);
 		break;
 
 	default:
@@ -209,7 +268,7 @@ static int ds2782_battery_get_property(struct power_supply *psy,
 	return ret;
 }
 
-static enum power_supply_property ds2782_battery_props[] = {
+static enum power_supply_property ds278x_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_CAPACITY,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -217,18 +276,18 @@ static enum power_supply_property ds2782_battery_props[] = {
 	POWER_SUPPLY_PROP_TEMP,
 };
 
-static void ds2782_power_supply_init(struct power_supply *battery)
+static void ds278x_power_supply_init(struct power_supply *battery)
 {
 	battery->type			= POWER_SUPPLY_TYPE_BATTERY;
-	battery->properties		= ds2782_battery_props;
-	battery->num_properties		= ARRAY_SIZE(ds2782_battery_props);
-	battery->get_property		= ds2782_battery_get_property;
+	battery->properties		= ds278x_battery_props;
+	battery->num_properties		= ARRAY_SIZE(ds278x_battery_props);
+	battery->get_property		= ds278x_battery_get_property;
 	battery->external_power_changed	= NULL;
 }
 
-static int ds2782_battery_remove(struct i2c_client *client)
+static int ds278x_battery_remove(struct i2c_client *client)
 {
-	struct ds2782_info *info = i2c_get_clientdata(client);
+	struct ds278x_info *info = i2c_get_clientdata(client);
 
 	power_supply_unregister(&info->battery);
 	kfree(info->battery.name);
@@ -243,13 +302,36 @@ static int ds2782_battery_remove(struct i2c_client *client)
 	return 0;
 }
 
-static int ds2782_battery_probe(struct i2c_client *client,
+static struct ds278x_battery_ops ds278x_ops[] = {
+	[0] = {
+		.get_current  = ds2782_get_current,
+		.get_voltage  = ds2782_get_voltage,
+		.get_capacity = ds2782_get_capacity,
+	},
+	[1] = {
+		.get_current  = ds2786_get_current,
+		.get_voltage  = ds2786_get_voltage,
+		.get_capacity = ds2786_get_capacity,
+	}
+};
+
+static int ds278x_battery_probe(struct i2c_client *client,
 				const struct i2c_device_id *id)
 {
-	struct ds2782_info *info;
+	struct ds278x_platform_data *pdata = client->dev.platform_data;
+	struct ds278x_info *info;
 	int ret;
 	int num;
 
+	/*
+	 * ds2786 should have the sense resistor value set
+	 * in the platform data
+	 */
+	if (id->driver_data == 1 && pdata == 0) {
+		dev_err(&client->dev, "missing platform data for ds2786\n");
+		return -EINVAL;
+	}
+
 	/* Get an ID for this battery */
 	ret = idr_pre_get(&battery_id, GFP_KERNEL);
 	if (ret == 0) {
@@ -269,15 +351,20 @@ static int ds2782_battery_probe(struct i2c_client *client,
 		goto fail_info;
 	}
 
-	info->battery.name = kasprintf(GFP_KERNEL, "ds2782-%d", num);
+	info->battery.name = kasprintf(GFP_KERNEL, "%s-%d", client->name, num);
 	if (!info->battery.name) {
 		ret = -ENOMEM;
 		goto fail_name;
 	}
 
+	if (id->driver_data == 1)
+		info->rsns = pdata->rsns;
+
 	i2c_set_clientdata(client, info);
 	info->client = client;
-	ds2782_power_supply_init(&info->battery);
+	info->id = num;
+	info->ops  = &ds278x_ops[id->driver_data];
+	ds278x_power_supply_init(&info->battery);
 
 	ret = power_supply_register(&client->dev, &info->battery);
 	if (ret) {
@@ -300,31 +387,32 @@ fail_id:
 	return ret;
 }
 
-static const struct i2c_device_id ds2782_id[] = {
+static const struct i2c_device_id ds278x_id[] = {
 	{"ds2782", 0},
+	{"ds2786", 1},
 	{},
 };
 
-static struct i2c_driver ds2782_battery_driver = {
+static struct i2c_driver ds278x_battery_driver = {
 	.driver 	= {
 		.name	= "ds2782-battery",
 	},
-	.probe		= ds2782_battery_probe,
-	.remove		= ds2782_battery_remove,
-	.id_table	= ds2782_id,
+	.probe		= ds278x_battery_probe,
+	.remove		= ds278x_battery_remove,
+	.id_table	= ds278x_id,
 };
 
-static int __init ds2782_init(void)
+static int __init ds278x_init(void)
 {
-	return i2c_add_driver(&ds2782_battery_driver);
+	return i2c_add_driver(&ds278x_battery_driver);
 }
-module_init(ds2782_init);
+module_init(ds278x_init);
 
-static void __exit ds2782_exit(void)
+static void __exit ds278x_exit(void)
 {
-	i2c_del_driver(&ds2782_battery_driver);
+	i2c_del_driver(&ds278x_battery_driver);
 }
-module_exit(ds2782_exit);
+module_exit(ds278x_exit);
 
 MODULE_AUTHOR("Ryan Mallon <ryan@bluewatersys.com>");
 MODULE_DESCRIPTION("Maxim/Dallas DS2782 Stand-Alone Fuel Gauage IC driver");
diff --git a/include/linux/ds2782_battery.h b/include/linux/ds2782_battery.h
new file mode 100644
index 0000000..b4e281f
--- /dev/null
+++ b/include/linux/ds2782_battery.h
@@ -0,0 +1,8 @@
+#ifndef __LINUX_DS2782_BATTERY_H
+#define __LINUX_DS2782_BATTERY_H
+
+struct ds278x_platform_data {
+	int rsns;
+};
+
+#endif
-- 
1.6.6.2


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

* Re: [PATCH v3] ds2782_battery: add support for ds2786 battery gas gauge
  2010-04-26 11:05 [PATCH v3] ds2782_battery: add support for ds2786 battery gas gauge Mike Rapoport
@ 2010-04-26 18:22 ` Anton Vorontsov
  2010-04-26 19:39   ` Mike Rapoport
  0 siblings, 1 reply; 4+ messages in thread
From: Anton Vorontsov @ 2010-04-26 18:22 UTC (permalink / raw)
  To: Mike Rapoport; +Cc: Ryan Mallon, linux-kernel, Yulia Vilensky

On Mon, Apr 26, 2010 at 02:05:25PM +0300, Mike Rapoport wrote:
> From: Yulia Vilensky <vilensky@compulab.co.il>
> 
> v2 changes:
>  * add ds278x_platform_data
>  * address Anton comments
> v3 changes:
>  * use ds278x_get_temp for both ds2782 and ds2786
>  * update math as per Ryan comments
> 
> Signed-off-by: Yulia Vilensky <vilensky@compulab.co.il>
> Signed-off-by: Mike Rapoport <mike@compulab.co.il>

Thanks, applied to battery-2.6 with the following change
(we shouldn't compare pointers and integers):

diff --git a/drivers/power/ds2782_battery.c b/drivers/power/ds2782_battery.c
index 6b3cee0..c665e80 100644
--- a/drivers/power/ds2782_battery.c
+++ b/drivers/power/ds2782_battery.c
@@ -325,7 +325,7 @@ static int ds278x_battery_probe(struct i2c_client *client,
 	 * ds2786 should have the sense resistor value set
 	 * in the platform data
 	 */
-	if (id->driver_data == 1 && pdata == 0) {
+	if (id->driver_data == 1 && !pdata) {
 		dev_err(&client->dev, "missing platform data for ds2786\n");
 		return -EINVAL;
 	}

----

Btw, I don't quite like the 'if (id->driver_data == 1)' stuff.
How about the following patch on top?

>From acf917d3880465b76875f671ee450a8fdff62c9f Mon Sep 17 00:00:00 2001
From: Anton Vorontsov <cbouatmailru@gmail.com>
Date: Mon, 26 Apr 2010 22:10:52 +0400
Subject: [PATCH] ds2782_battery: Get rid of magic numbers in driver_data

Constructions like 'if (id->driver_data == 1)' look quite weird.
This patch introduces 'enum ds278x_num_id', which makes things
much more understandable, i.e. 'if (id->driver_data == DS2786)'.

Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
 drivers/power/ds2782_battery.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/power/ds2782_battery.c b/drivers/power/ds2782_battery.c
index c665e80..d762a0c 100644
--- a/drivers/power/ds2782_battery.c
+++ b/drivers/power/ds2782_battery.c
@@ -300,13 +300,18 @@ static int ds278x_battery_remove(struct i2c_client *client)
 	return 0;
 }
 
+enum ds278x_num_id {
+	DS2782 = 0,
+	DS2786,
+};
+
 static struct ds278x_battery_ops ds278x_ops[] = {
-	[0] = {
+	[DS2782] = {
 		.get_current  = ds2782_get_current,
 		.get_voltage  = ds2782_get_voltage,
 		.get_capacity = ds2782_get_capacity,
 	},
-	[1] = {
+	[DS2786] = {
 		.get_current  = ds2786_get_current,
 		.get_voltage  = ds2786_get_voltage,
 		.get_capacity = ds2786_get_capacity,
@@ -325,7 +330,7 @@ static int ds278x_battery_probe(struct i2c_client *client,
 	 * ds2786 should have the sense resistor value set
 	 * in the platform data
 	 */
-	if (id->driver_data == 1 && !pdata) {
+	if (id->driver_data == DS2786 && !pdata) {
 		dev_err(&client->dev, "missing platform data for ds2786\n");
 		return -EINVAL;
 	}
@@ -355,7 +360,7 @@ static int ds278x_battery_probe(struct i2c_client *client,
 		goto fail_name;
 	}
 
-	if (id->driver_data == 1)
+	if (id->driver_data == DS2786)
 		info->rsns = pdata->rsns;
 
 	i2c_set_clientdata(client, info);
@@ -385,8 +390,8 @@ fail_id:
 }
 
 static const struct i2c_device_id ds278x_id[] = {
-	{"ds2782", 0},
-	{"ds2786", 1},
+	{"ds2782", DS2782},
+	{"ds2786", DS2786},
 	{},
 };
 
-- 
1.7.0.5


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

* Re: [PATCH v3] ds2782_battery: add support for ds2786 battery gas  gauge
  2010-04-26 18:22 ` Anton Vorontsov
@ 2010-04-26 19:39   ` Mike Rapoport
  2010-05-02 20:12     ` Anton Vorontsov
  0 siblings, 1 reply; 4+ messages in thread
From: Mike Rapoport @ 2010-04-26 19:39 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: Mike Rapoport, Ryan Mallon, linux-kernel, Yulia Vilensky

On Mon, Apr 26, 2010 at 9:22 PM, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
>
> Btw, I don't quite like the 'if (id->driver_data == 1)' stuff.
> How about the following patch on top?
>
> From acf917d3880465b76875f671ee450a8fdff62c9f Mon Sep 17 00:00:00 2001
> From: Anton Vorontsov <cbouatmailru@gmail.com>
> Date: Mon, 26 Apr 2010 22:10:52 +0400
> Subject: [PATCH] ds2782_battery: Get rid of magic numbers in driver_data
>
> Constructions like 'if (id->driver_data == 1)' look quite weird.
> This patch introduces 'enum ds278x_num_id', which makes things
> much more understandable, i.e. 'if (id->driver_data == DS2786)'.

agree

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

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

> ---
>  drivers/power/ds2782_battery.c |   17 +++++++++++------
>  1 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/power/ds2782_battery.c b/drivers/power/ds2782_battery.c
> index c665e80..d762a0c 100644
> --- a/drivers/power/ds2782_battery.c
> +++ b/drivers/power/ds2782_battery.c
> @@ -300,13 +300,18 @@ static int ds278x_battery_remove(struct i2c_client *client)
>        return 0;
>  }
>
> +enum ds278x_num_id {
> +       DS2782 = 0,
> +       DS2786,
> +};
> +
>  static struct ds278x_battery_ops ds278x_ops[] = {
> -       [0] = {
> +       [DS2782] = {
>                .get_current  = ds2782_get_current,
>                .get_voltage  = ds2782_get_voltage,
>                .get_capacity = ds2782_get_capacity,
>        },
> -       [1] = {
> +       [DS2786] = {
>                .get_current  = ds2786_get_current,
>                .get_voltage  = ds2786_get_voltage,
>                .get_capacity = ds2786_get_capacity,
> @@ -325,7 +330,7 @@ static int ds278x_battery_probe(struct i2c_client *client,
>         * ds2786 should have the sense resistor value set
>         * in the platform data
>         */
> -       if (id->driver_data == 1 && !pdata) {
> +       if (id->driver_data == DS2786 && !pdata) {
>                dev_err(&client->dev, "missing platform data for ds2786\n");
>                return -EINVAL;
>        }
> @@ -355,7 +360,7 @@ static int ds278x_battery_probe(struct i2c_client *client,
>                goto fail_name;
>        }
>
> -       if (id->driver_data == 1)
> +       if (id->driver_data == DS2786)
>                info->rsns = pdata->rsns;
>
>        i2c_set_clientdata(client, info);
> @@ -385,8 +390,8 @@ fail_id:
>  }
>
>  static const struct i2c_device_id ds278x_id[] = {
> -       {"ds2782", 0},
> -       {"ds2786", 1},
> +       {"ds2782", DS2782},
> +       {"ds2786", DS2786},
>        {},
>  };
>
> --
> 1.7.0.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>



-- 
	Sincerely Yours,
		Mike.

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

* Re: [PATCH v3] ds2782_battery: add support for ds2786 battery gas gauge
  2010-04-26 19:39   ` Mike Rapoport
@ 2010-05-02 20:12     ` Anton Vorontsov
  0 siblings, 0 replies; 4+ messages in thread
From: Anton Vorontsov @ 2010-05-02 20:12 UTC (permalink / raw)
  To: Mike Rapoport; +Cc: Mike Rapoport, Ryan Mallon, linux-kernel, Yulia Vilensky

On Mon, Apr 26, 2010 at 10:39:41PM +0300, Mike Rapoport wrote:
> On Mon, Apr 26, 2010 at 9:22 PM, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
> >
> > Btw, I don't quite like the 'if (id->driver_data == 1)' stuff.
> > How about the following patch on top?
> >
> > From acf917d3880465b76875f671ee450a8fdff62c9f Mon Sep 17 00:00:00 2001
> > From: Anton Vorontsov <cbouatmailru@gmail.com>
> > Date: Mon, 26 Apr 2010 22:10:52 +0400
> > Subject: [PATCH] ds2782_battery: Get rid of magic numbers in driver_data
> >
> > Constructions like 'if (id->driver_data == 1)' look quite weird.
> > This patch introduces 'enum ds278x_num_id', which makes things
> > much more understandable, i.e. 'if (id->driver_data == DS2786)'.
> 
> agree
> 
> > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> 
> Acked-by: Mike Rapoport <mike@compulab.co.il>

Thanks! Applied.

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

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

end of thread, other threads:[~2010-05-02 20:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-26 11:05 [PATCH v3] ds2782_battery: add support for ds2786 battery gas gauge Mike Rapoport
2010-04-26 18:22 ` Anton Vorontsov
2010-04-26 19:39   ` Mike Rapoport
2010-05-02 20:12     ` Anton Vorontsov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox