* [PATCH v2] add MAX17040 Fuel Gauge driver
@ 2009-06-04 8:55 ` Minkyu Kang
0 siblings, 0 replies; 18+ messages in thread
From: Minkyu Kang @ 2009-06-04 8:55 UTC (permalink / raw)
To: linux-kernel, linux-pm; +Cc: kyungmin.park, Anton Vorontsov, Andrew Morton
The MAX17040 is a I2C interfaced Fuel Gauge systems for lithium-ion batteries
This patch adds support the MAX17040 Fuel Gauge
Cc: Anton Vorontsov <cbou@mail.ru>
Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
---
drivers/power/Kconfig | 8 +
drivers/power/Makefile | 3 +-
drivers/power/max17040_battery.c | 306 ++++++++++++++++++++++++++++++++++++++
include/linux/max17040_battery.h | 19 +++
4 files changed, 335 insertions(+), 1 deletions(-)
create mode 100644 drivers/power/max17040_battery.c
create mode 100644 include/linux/max17040_battery.h
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 33da112..6af5798 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -88,4 +88,12 @@ config CHARGER_PCF50633
help
Say Y to include support for NXP PCF50633 Main Battery Charger.
+config BATTERY_MAX17040
+ tristate "Maxim MAX17040 Fuel Gauge"
+ depends on I2C
+ help
+ MAX17040 is fuel-gauge systems for lithium-ion (Li+) batteries
+ in handheld and portable equipment. The MAX17040 is configured
+ to operate with a single lithium cell
+
endif # POWER_SUPPLY
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 2fcf41d..9c48995 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -25,4 +25,5 @@ obj-$(CONFIG_BATTERY_TOSA) += tosa_battery.o
obj-$(CONFIG_BATTERY_WM97XX) += wm97xx_battery.o
obj-$(CONFIG_BATTERY_BQ27x00) += bq27x00_battery.o
obj-$(CONFIG_BATTERY_DA9030) += da9030_battery.o
-obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
\ No newline at end of file
+obj-$(CONFIG_BATTERY_MAX17040)) += max17040_battery.o
+obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
diff --git a/drivers/power/max17040_battery.c b/drivers/power/max17040_battery.c
new file mode 100644
index 0000000..25303ba
--- /dev/null
+++ b/drivers/power/max17040_battery.c
@@ -0,0 +1,306 @@
+/*
+ * max17040_battery.c
+ * fuel-gauge systems for lithium-ion (Li+) batteries
+ *
+ * Copyright (C) 2009 Samsung Electronics
+ * Minkyu Kang <mk7.kang@samsung.com>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/mutex.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/power_supply.h>
+#include <linux/max17040_battery.h>
+
+#define MAX17040_VCELL_MSB 0x02
+#define MAX17040_VCELL_LSB 0x03
+#define MAX17040_SOC_MSB 0x04
+#define MAX17040_SOC_LSB 0x05
+#define MAX17040_MODE_MSB 0x06
+#define MAX17040_MODE_LSB 0x07
+#define MAX17040_VER_MSB 0x08
+#define MAX17040_VER_LSB 0x09
+#define MAX17040_RCOMP_MSB 0x0C
+#define MAX17040_RCOMP_LSB 0x0D
+#define MAX17040_CMD_MSB 0xFE
+#define MAX17040_CMD_LSB 0xFF
+
+#define MAX17040_DELAY 1000
+#define MAX17040_BATTERY_FULL 95
+
+struct max17040_chip {
+ struct i2c_client *client;
+ struct delayed_work work;
+
+ /* State Of Connect */
+ int online;
+ /* battery voltage */
+ int vcell;
+ /* battery capacity */
+ int soc;
+ /* State Of Charge */
+ int status;
+};
+
+static struct max17040_chip *max17040;
+static struct max17040_platform_data *pdata;
+
+static int max17040_get_property(struct power_supply *bat_ps,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ switch (psp) {
+ case POWER_SUPPLY_PROP_STATUS:
+ val->intval = max17040->status;
+ break;
+ case POWER_SUPPLY_PROP_ONLINE:
+ val->intval = max17040->online;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ val->intval = max17040->vcell;
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ val->intval = max17040->soc;
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static struct power_supply bat_ps = {
+ .name = "battery",
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .get_property = max17040_get_property,
+};
+
+static int max17040_write_reg(int reg, u8 value)
+{
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(max17040->client, reg, value);
+
+ if (ret < 0) {
+ dev_err(&max17040->client->dev,
+ "%s: reg 0x%x, val 0x%x, err %d\n",
+ __func__, reg, value, ret);
+ }
+
+ return ret;
+}
+
+static u8 max17040_read_reg(int reg)
+{
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(max17040->client, reg);
+
+ if (ret < 0) {
+ dev_err(&max17040->client->dev,
+ "%s: reg 0x%x, err %d\n",
+ __func__, reg, ret);
+ }
+
+ return ret;
+}
+
+static void max17040_reset(void)
+{
+ max17040_write_reg(MAX17040_CMD_MSB, 0x54);
+ max17040_write_reg(MAX17040_CMD_LSB, 0x00);
+}
+
+static void max17040_set_rcomp(u16 val)
+{
+ max17040_write_reg(MAX17040_RCOMP_MSB, val >> 8);
+ max17040_write_reg(MAX17040_RCOMP_LSB, (val << 8) >> 8);
+}
+
+static void max17040_get_vcell(void)
+{
+ u8 msb;
+ u8 lsb;
+
+ msb = max17040_read_reg(MAX17040_VCELL_MSB);
+ lsb = max17040_read_reg(MAX17040_VCELL_LSB);
+
+ max17040->vcell = (msb << 4) + (lsb >> 4);
+}
+
+static void max17040_get_soc(void)
+{
+ u8 msb;
+ u8 lsb;
+
+ msb = max17040_read_reg(MAX17040_SOC_MSB);
+ lsb = max17040_read_reg(MAX17040_SOC_LSB);
+
+ max17040->soc = msb;
+}
+
+static void max17040_get_version(void)
+{
+ u8 msb;
+ u8 lsb;
+
+ msb = max17040_read_reg(MAX17040_VER_MSB);
+ lsb = max17040_read_reg(MAX17040_VER_LSB);
+
+ dev_info(&max17040->client->dev,
+ "MAX17040 Fuel-Gauge Ver %d%d\n", msb, lsb);
+}
+
+static void max17040_get_online(void)
+{
+ if (pdata->battery_online)
+ max17040->online = pdata->battery_online();
+ else
+ max17040->online = 1;
+}
+
+static void max17040_get_status(void)
+{
+ if (!pdata->charger_online || !pdata->charger_enable) {
+ max17040->status = POWER_SUPPLY_STATUS_UNKNOWN;
+ return;
+ }
+
+ if (pdata->charger_online()) {
+ if (pdata->charger_enable())
+ max17040->status = POWER_SUPPLY_STATUS_CHARGING;
+ else
+ max17040->status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ } else {
+ max17040->status = POWER_SUPPLY_STATUS_DISCHARGING;
+ }
+
+ if (max17040->soc > MAX17040_BATTERY_FULL)
+ max17040->status = POWER_SUPPLY_STATUS_FULL;
+}
+
+static void max17040_work(struct work_struct *work)
+{
+ max17040_get_vcell();
+ max17040_get_soc();
+ max17040_get_online();
+ max17040_get_status();
+
+ schedule_delayed_work(&max17040->work, MAX17040_DELAY);
+}
+
+static enum power_supply_property max17040_battery_props[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int __devinit max17040_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct max17040_chip *chip;
+ int ret;
+
+ chip = kzalloc(sizeof(struct max17040_chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->client = client;
+ pdata = client->dev.platform_data;
+
+ i2c_set_clientdata(client, chip);
+ max17040 = chip;
+
+ max17040_reset();
+
+ max17040_get_version();
+
+ INIT_DELAYED_WORK_DEFERRABLE(&chip->work, max17040_work);
+ schedule_delayed_work(&chip->work, MAX17040_DELAY);
+
+ bat_ps.properties = max17040_battery_props;
+ bat_ps.num_properties = ARRAY_SIZE(max17040_battery_props);
+
+ ret = power_supply_register(&client->dev, &bat_ps);
+ if (ret) {
+ dev_err(&max17040->client->dev,
+ "failed: power supply register\n");
+ cancel_delayed_work(&chip->work);
+ i2c_set_clientdata(client, NULL);
+ kfree(chip);
+ max17040 = NULL;
+ return -1;
+ }
+
+ return 0;
+}
+
+static int __devexit max17040_remove(struct i2c_client *client)
+{
+ struct max17040_chip *chip = i2c_get_clientdata(client);
+
+ power_supply_unregister(&bat_ps);
+ cancel_delayed_work(&chip->work);
+ i2c_set_clientdata(client, NULL);
+ kfree(chip);
+ max17040 = NULL;
+ return 0;
+}
+
+static int max17040_suspend(struct i2c_client *client,
+ pm_message_t state)
+{
+ struct max17040_chip *chip = i2c_get_clientdata(client);
+
+ cancel_delayed_work(&chip->work);
+ return 0;
+}
+
+static int max17040_resume(struct i2c_client *client)
+{
+ struct max17040_chip *chip = i2c_get_clientdata(client);
+
+ schedule_delayed_work(&chip->work, MAX17040_DELAY);
+ return 0;
+}
+
+static const struct i2c_device_id max17040_id[] = {
+ { "max17040", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max17040_id);
+
+static struct i2c_driver max17040_i2c_driver = {
+ .driver = {
+ .name = "max17040",
+ },
+ .probe = max17040_probe,
+ .remove = __devexit_p(max17040_remove),
+ .suspend = max17040_suspend,
+ .resume = max17040_resume,
+ .id_table = max17040_id,
+};
+
+static int __init max17040_init(void)
+{
+ return i2c_add_driver(&max17040_i2c_driver);
+}
+module_init(max17040_init);
+
+static void __exit max17040_exit(void)
+{
+ i2c_del_driver(&max17040_i2c_driver);
+}
+module_exit(max17040_exit);
+
+MODULE_AUTHOR("Minkyu Kang <mk7.kang@samsung.com>");
+MODULE_DESCRIPTION("MAX17040 Fuel Gauge");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/max17040_battery.h b/include/linux/max17040_battery.h
new file mode 100644
index 0000000..ad97b06
--- /dev/null
+++ b/include/linux/max17040_battery.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2009 Samsung Electronics
+ * Minkyu Kang <mk7.kang@samsung.com>
+ *
+ * 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.
+ */
+
+#ifndef __MAX17040_BATTERY_H_
+#define __MAX17040_BATTERY_H_
+
+struct max17040_platform_data {
+ int (*battery_online)(void);
+ int (*charger_online)(void);
+ int (*charger_enable)(void);
+};
+
+#endif
--
1.5.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2] add MAX17040 Fuel Gauge driver
@ 2009-06-04 8:55 ` Minkyu Kang
0 siblings, 0 replies; 18+ messages in thread
From: Minkyu Kang @ 2009-06-04 8:55 UTC (permalink / raw)
To: linux-kernel, linux-pm; +Cc: Anton Vorontsov, kyungmin.park, Andrew Morton
The MAX17040 is a I2C interfaced Fuel Gauge systems for lithium-ion batteries
This patch adds support the MAX17040 Fuel Gauge
Cc: Anton Vorontsov <cbou@mail.ru>
Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
---
drivers/power/Kconfig | 8 +
drivers/power/Makefile | 3 +-
drivers/power/max17040_battery.c | 306 ++++++++++++++++++++++++++++++++++++++
include/linux/max17040_battery.h | 19 +++
4 files changed, 335 insertions(+), 1 deletions(-)
create mode 100644 drivers/power/max17040_battery.c
create mode 100644 include/linux/max17040_battery.h
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 33da112..6af5798 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -88,4 +88,12 @@ config CHARGER_PCF50633
help
Say Y to include support for NXP PCF50633 Main Battery Charger.
+config BATTERY_MAX17040
+ tristate "Maxim MAX17040 Fuel Gauge"
+ depends on I2C
+ help
+ MAX17040 is fuel-gauge systems for lithium-ion (Li+) batteries
+ in handheld and portable equipment. The MAX17040 is configured
+ to operate with a single lithium cell
+
endif # POWER_SUPPLY
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 2fcf41d..9c48995 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -25,4 +25,5 @@ obj-$(CONFIG_BATTERY_TOSA) += tosa_battery.o
obj-$(CONFIG_BATTERY_WM97XX) += wm97xx_battery.o
obj-$(CONFIG_BATTERY_BQ27x00) += bq27x00_battery.o
obj-$(CONFIG_BATTERY_DA9030) += da9030_battery.o
-obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
\ No newline at end of file
+obj-$(CONFIG_BATTERY_MAX17040)) += max17040_battery.o
+obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
diff --git a/drivers/power/max17040_battery.c b/drivers/power/max17040_battery.c
new file mode 100644
index 0000000..25303ba
--- /dev/null
+++ b/drivers/power/max17040_battery.c
@@ -0,0 +1,306 @@
+/*
+ * max17040_battery.c
+ * fuel-gauge systems for lithium-ion (Li+) batteries
+ *
+ * Copyright (C) 2009 Samsung Electronics
+ * Minkyu Kang <mk7.kang@samsung.com>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/mutex.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/power_supply.h>
+#include <linux/max17040_battery.h>
+
+#define MAX17040_VCELL_MSB 0x02
+#define MAX17040_VCELL_LSB 0x03
+#define MAX17040_SOC_MSB 0x04
+#define MAX17040_SOC_LSB 0x05
+#define MAX17040_MODE_MSB 0x06
+#define MAX17040_MODE_LSB 0x07
+#define MAX17040_VER_MSB 0x08
+#define MAX17040_VER_LSB 0x09
+#define MAX17040_RCOMP_MSB 0x0C
+#define MAX17040_RCOMP_LSB 0x0D
+#define MAX17040_CMD_MSB 0xFE
+#define MAX17040_CMD_LSB 0xFF
+
+#define MAX17040_DELAY 1000
+#define MAX17040_BATTERY_FULL 95
+
+struct max17040_chip {
+ struct i2c_client *client;
+ struct delayed_work work;
+
+ /* State Of Connect */
+ int online;
+ /* battery voltage */
+ int vcell;
+ /* battery capacity */
+ int soc;
+ /* State Of Charge */
+ int status;
+};
+
+static struct max17040_chip *max17040;
+static struct max17040_platform_data *pdata;
+
+static int max17040_get_property(struct power_supply *bat_ps,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ switch (psp) {
+ case POWER_SUPPLY_PROP_STATUS:
+ val->intval = max17040->status;
+ break;
+ case POWER_SUPPLY_PROP_ONLINE:
+ val->intval = max17040->online;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ val->intval = max17040->vcell;
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ val->intval = max17040->soc;
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static struct power_supply bat_ps = {
+ .name = "battery",
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .get_property = max17040_get_property,
+};
+
+static int max17040_write_reg(int reg, u8 value)
+{
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(max17040->client, reg, value);
+
+ if (ret < 0) {
+ dev_err(&max17040->client->dev,
+ "%s: reg 0x%x, val 0x%x, err %d\n",
+ __func__, reg, value, ret);
+ }
+
+ return ret;
+}
+
+static u8 max17040_read_reg(int reg)
+{
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(max17040->client, reg);
+
+ if (ret < 0) {
+ dev_err(&max17040->client->dev,
+ "%s: reg 0x%x, err %d\n",
+ __func__, reg, ret);
+ }
+
+ return ret;
+}
+
+static void max17040_reset(void)
+{
+ max17040_write_reg(MAX17040_CMD_MSB, 0x54);
+ max17040_write_reg(MAX17040_CMD_LSB, 0x00);
+}
+
+static void max17040_set_rcomp(u16 val)
+{
+ max17040_write_reg(MAX17040_RCOMP_MSB, val >> 8);
+ max17040_write_reg(MAX17040_RCOMP_LSB, (val << 8) >> 8);
+}
+
+static void max17040_get_vcell(void)
+{
+ u8 msb;
+ u8 lsb;
+
+ msb = max17040_read_reg(MAX17040_VCELL_MSB);
+ lsb = max17040_read_reg(MAX17040_VCELL_LSB);
+
+ max17040->vcell = (msb << 4) + (lsb >> 4);
+}
+
+static void max17040_get_soc(void)
+{
+ u8 msb;
+ u8 lsb;
+
+ msb = max17040_read_reg(MAX17040_SOC_MSB);
+ lsb = max17040_read_reg(MAX17040_SOC_LSB);
+
+ max17040->soc = msb;
+}
+
+static void max17040_get_version(void)
+{
+ u8 msb;
+ u8 lsb;
+
+ msb = max17040_read_reg(MAX17040_VER_MSB);
+ lsb = max17040_read_reg(MAX17040_VER_LSB);
+
+ dev_info(&max17040->client->dev,
+ "MAX17040 Fuel-Gauge Ver %d%d\n", msb, lsb);
+}
+
+static void max17040_get_online(void)
+{
+ if (pdata->battery_online)
+ max17040->online = pdata->battery_online();
+ else
+ max17040->online = 1;
+}
+
+static void max17040_get_status(void)
+{
+ if (!pdata->charger_online || !pdata->charger_enable) {
+ max17040->status = POWER_SUPPLY_STATUS_UNKNOWN;
+ return;
+ }
+
+ if (pdata->charger_online()) {
+ if (pdata->charger_enable())
+ max17040->status = POWER_SUPPLY_STATUS_CHARGING;
+ else
+ max17040->status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ } else {
+ max17040->status = POWER_SUPPLY_STATUS_DISCHARGING;
+ }
+
+ if (max17040->soc > MAX17040_BATTERY_FULL)
+ max17040->status = POWER_SUPPLY_STATUS_FULL;
+}
+
+static void max17040_work(struct work_struct *work)
+{
+ max17040_get_vcell();
+ max17040_get_soc();
+ max17040_get_online();
+ max17040_get_status();
+
+ schedule_delayed_work(&max17040->work, MAX17040_DELAY);
+}
+
+static enum power_supply_property max17040_battery_props[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int __devinit max17040_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct max17040_chip *chip;
+ int ret;
+
+ chip = kzalloc(sizeof(struct max17040_chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->client = client;
+ pdata = client->dev.platform_data;
+
+ i2c_set_clientdata(client, chip);
+ max17040 = chip;
+
+ max17040_reset();
+
+ max17040_get_version();
+
+ INIT_DELAYED_WORK_DEFERRABLE(&chip->work, max17040_work);
+ schedule_delayed_work(&chip->work, MAX17040_DELAY);
+
+ bat_ps.properties = max17040_battery_props;
+ bat_ps.num_properties = ARRAY_SIZE(max17040_battery_props);
+
+ ret = power_supply_register(&client->dev, &bat_ps);
+ if (ret) {
+ dev_err(&max17040->client->dev,
+ "failed: power supply register\n");
+ cancel_delayed_work(&chip->work);
+ i2c_set_clientdata(client, NULL);
+ kfree(chip);
+ max17040 = NULL;
+ return -1;
+ }
+
+ return 0;
+}
+
+static int __devexit max17040_remove(struct i2c_client *client)
+{
+ struct max17040_chip *chip = i2c_get_clientdata(client);
+
+ power_supply_unregister(&bat_ps);
+ cancel_delayed_work(&chip->work);
+ i2c_set_clientdata(client, NULL);
+ kfree(chip);
+ max17040 = NULL;
+ return 0;
+}
+
+static int max17040_suspend(struct i2c_client *client,
+ pm_message_t state)
+{
+ struct max17040_chip *chip = i2c_get_clientdata(client);
+
+ cancel_delayed_work(&chip->work);
+ return 0;
+}
+
+static int max17040_resume(struct i2c_client *client)
+{
+ struct max17040_chip *chip = i2c_get_clientdata(client);
+
+ schedule_delayed_work(&chip->work, MAX17040_DELAY);
+ return 0;
+}
+
+static const struct i2c_device_id max17040_id[] = {
+ { "max17040", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max17040_id);
+
+static struct i2c_driver max17040_i2c_driver = {
+ .driver = {
+ .name = "max17040",
+ },
+ .probe = max17040_probe,
+ .remove = __devexit_p(max17040_remove),
+ .suspend = max17040_suspend,
+ .resume = max17040_resume,
+ .id_table = max17040_id,
+};
+
+static int __init max17040_init(void)
+{
+ return i2c_add_driver(&max17040_i2c_driver);
+}
+module_init(max17040_init);
+
+static void __exit max17040_exit(void)
+{
+ i2c_del_driver(&max17040_i2c_driver);
+}
+module_exit(max17040_exit);
+
+MODULE_AUTHOR("Minkyu Kang <mk7.kang@samsung.com>");
+MODULE_DESCRIPTION("MAX17040 Fuel Gauge");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/max17040_battery.h b/include/linux/max17040_battery.h
new file mode 100644
index 0000000..ad97b06
--- /dev/null
+++ b/include/linux/max17040_battery.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2009 Samsung Electronics
+ * Minkyu Kang <mk7.kang@samsung.com>
+ *
+ * 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.
+ */
+
+#ifndef __MAX17040_BATTERY_H_
+#define __MAX17040_BATTERY_H_
+
+struct max17040_platform_data {
+ int (*battery_online)(void);
+ int (*charger_online)(void);
+ int (*charger_enable)(void);
+};
+
+#endif
--
1.5.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
2009-06-04 8:55 ` Minkyu Kang
(?)
(?)
@ 2009-06-04 9:06 ` Andrew Morton
2009-06-04 10:39 ` Minkyu Kang
2009-06-04 10:39 ` Minkyu Kang
-1 siblings, 2 replies; 18+ messages in thread
From: Andrew Morton @ 2009-06-04 9:06 UTC (permalink / raw)
To: Minkyu Kang; +Cc: linux-kernel, linux-pm, kyungmin.park, Anton Vorontsov
On Thu, 04 Jun 2009 17:55:36 +0900 Minkyu Kang <mk7.kang@samsung.com> wrote:
> + INIT_DELAYED_WORK_DEFERRABLE(&chip->work, max17040_work);
> + schedule_delayed_work(&chip->work, MAX17040_DELAY);
> +
> + bat_ps.properties = max17040_battery_props;
> + bat_ps.num_properties = ARRAY_SIZE(max17040_battery_props);
> +
> + ret = power_supply_register(&client->dev, &bat_ps);
> + if (ret) {
> + dev_err(&max17040->client->dev,
> + "failed: power supply register\n");
> + cancel_delayed_work(&chip->work);
> + i2c_set_clientdata(client, NULL);
> + kfree(chip);
> + max17040 = NULL;
> + return -1;
> + }
> +
> + return 0;
> +}
Wouldn't it be better to start the delayed_work after the
power_supply_register() has succeeded?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
2009-06-04 8:55 ` Minkyu Kang
(?)
@ 2009-06-04 9:06 ` Andrew Morton
-1 siblings, 0 replies; 18+ messages in thread
From: Andrew Morton @ 2009-06-04 9:06 UTC (permalink / raw)
To: Minkyu Kang; +Cc: kyungmin.park, linux-pm, linux-kernel, Anton Vorontsov
On Thu, 04 Jun 2009 17:55:36 +0900 Minkyu Kang <mk7.kang@samsung.com> wrote:
> + INIT_DELAYED_WORK_DEFERRABLE(&chip->work, max17040_work);
> + schedule_delayed_work(&chip->work, MAX17040_DELAY);
> +
> + bat_ps.properties = max17040_battery_props;
> + bat_ps.num_properties = ARRAY_SIZE(max17040_battery_props);
> +
> + ret = power_supply_register(&client->dev, &bat_ps);
> + if (ret) {
> + dev_err(&max17040->client->dev,
> + "failed: power supply register\n");
> + cancel_delayed_work(&chip->work);
> + i2c_set_clientdata(client, NULL);
> + kfree(chip);
> + max17040 = NULL;
> + return -1;
> + }
> +
> + return 0;
> +}
Wouldn't it be better to start the delayed_work after the
power_supply_register() has succeeded?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
@ 2009-06-04 9:16 ` Trilok Soni
0 siblings, 0 replies; 18+ messages in thread
From: Trilok Soni @ 2009-06-04 9:16 UTC (permalink / raw)
To: Minkyu Kang
Cc: linux-kernel, linux-pm, kyungmin.park, Anton Vorontsov,
Andrew Morton, linux-i2c
Hi Minkyu Kang,
Adding linux-i2c mailing list, so not deleting any code.
On Thu, Jun 4, 2009 at 2:25 PM, Minkyu Kang <mk7.kang@samsung.com> wrote:
> The MAX17040 is a I2C interfaced Fuel Gauge systems for lithium-ion batteries
> This patch adds support the MAX17040 Fuel Gauge
>
> Cc: Anton Vorontsov <cbou@mail.ru>
> Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
> ---
> drivers/power/Kconfig | 8 +
> drivers/power/Makefile | 3 +-
> drivers/power/max17040_battery.c | 306 ++++++++++++++++++++++++++++++++++++++
> include/linux/max17040_battery.h | 19 +++
> 4 files changed, 335 insertions(+), 1 deletions(-)
> create mode 100644 drivers/power/max17040_battery.c
> create mode 100644 include/linux/max17040_battery.h
>
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 33da112..6af5798 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -88,4 +88,12 @@ config CHARGER_PCF50633
> help
> Say Y to include support for NXP PCF50633 Main Battery Charger.
>
> +config BATTERY_MAX17040
> + tristate "Maxim MAX17040 Fuel Gauge"
> + depends on I2C
> + help
> + MAX17040 is fuel-gauge systems for lithium-ion (Li+) batteries
> + in handheld and portable equipment. The MAX17040 is configured
> + to operate with a single lithium cell
> +
> endif # POWER_SUPPLY
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index 2fcf41d..9c48995 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -25,4 +25,5 @@ obj-$(CONFIG_BATTERY_TOSA) += tosa_battery.o
> obj-$(CONFIG_BATTERY_WM97XX) += wm97xx_battery.o
> obj-$(CONFIG_BATTERY_BQ27x00) += bq27x00_battery.o
> obj-$(CONFIG_BATTERY_DA9030) += da9030_battery.o
> -obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
> \ No newline at end of file
> +obj-$(CONFIG_BATTERY_MAX17040)) += max17040_battery.o
> +obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
> diff --git a/drivers/power/max17040_battery.c b/drivers/power/max17040_battery.c
> new file mode 100644
> index 0000000..25303ba
> --- /dev/null
> +++ b/drivers/power/max17040_battery.c
> @@ -0,0 +1,306 @@
> +/*
> + * max17040_battery.c
> + * fuel-gauge systems for lithium-ion (Li+) batteries
> + *
> + * Copyright (C) 2009 Samsung Electronics
> + * Minkyu Kang <mk7.kang@samsung.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/mutex.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/power_supply.h>
> +#include <linux/max17040_battery.h>
> +
> +#define MAX17040_VCELL_MSB 0x02
> +#define MAX17040_VCELL_LSB 0x03
> +#define MAX17040_SOC_MSB 0x04
> +#define MAX17040_SOC_LSB 0x05
> +#define MAX17040_MODE_MSB 0x06
> +#define MAX17040_MODE_LSB 0x07
> +#define MAX17040_VER_MSB 0x08
> +#define MAX17040_VER_LSB 0x09
> +#define MAX17040_RCOMP_MSB 0x0C
> +#define MAX17040_RCOMP_LSB 0x0D
> +#define MAX17040_CMD_MSB 0xFE
> +#define MAX17040_CMD_LSB 0xFF
> +
> +#define MAX17040_DELAY 1000
> +#define MAX17040_BATTERY_FULL 95
> +
> +struct max17040_chip {
> + struct i2c_client *client;
> + struct delayed_work work;
> +
> + /* State Of Connect */
> + int online;
> + /* battery voltage */
> + int vcell;
> + /* battery capacity */
> + int soc;
> + /* State Of Charge */
> + int status;
> +};
> +
> +static struct max17040_chip *max17040;
> +static struct max17040_platform_data *pdata;
May be you want to move this pdata under chip structure.
> +
> +static int max17040_get_property(struct power_supply *bat_ps,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + switch (psp) {
> + case POWER_SUPPLY_PROP_STATUS:
> + val->intval = max17040->status;
> + break;
> + case POWER_SUPPLY_PROP_ONLINE:
> + val->intval = max17040->online;
> + break;
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + val->intval = max17040->vcell;
> + break;
> + case POWER_SUPPLY_PROP_CAPACITY:
> + val->intval = max17040->soc;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static struct power_supply bat_ps = {
> + .name = "battery",
> + .type = POWER_SUPPLY_TYPE_BATTERY,
> + .get_property = max17040_get_property,
> +};
> +
> +static int max17040_write_reg(int reg, u8 value)
> +{
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(max17040->client, reg, value);
> +
> + if (ret < 0) {
> + dev_err(&max17040->client->dev,
> + "%s: reg 0x%x, val 0x%x, err %d\n",
> + __func__, reg, value, ret);
> + }
> +
> + return ret;
> +}
> +
> +static u8 max17040_read_reg(int reg)
> +{
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(max17040->client, reg);
> +
> + if (ret < 0) {
> + dev_err(&max17040->client->dev,
> + "%s: reg 0x%x, err %d\n",
> + __func__, reg, ret);
> + }
> +
> + return ret;
> +}
> +
> +static void max17040_reset(void)
> +{
> + max17040_write_reg(MAX17040_CMD_MSB, 0x54);
> + max17040_write_reg(MAX17040_CMD_LSB, 0x00);
> +}
> +
> +static void max17040_set_rcomp(u16 val)
> +{
> + max17040_write_reg(MAX17040_RCOMP_MSB, val >> 8);
> + max17040_write_reg(MAX17040_RCOMP_LSB, (val << 8) >> 8);
> +}
> +
> +static void max17040_get_vcell(void)
> +{
> + u8 msb;
> + u8 lsb;
> +
> + msb = max17040_read_reg(MAX17040_VCELL_MSB);
> + lsb = max17040_read_reg(MAX17040_VCELL_LSB);
> +
> + max17040->vcell = (msb << 4) + (lsb >> 4);
> +}
> +
> +static void max17040_get_soc(void)
> +{
> + u8 msb;
> + u8 lsb;
> +
> + msb = max17040_read_reg(MAX17040_SOC_MSB);
> + lsb = max17040_read_reg(MAX17040_SOC_LSB);
> +
> + max17040->soc = msb;
> +}
> +
> +static void max17040_get_version(void)
> +{
> + u8 msb;
> + u8 lsb;
> +
> + msb = max17040_read_reg(MAX17040_VER_MSB);
> + lsb = max17040_read_reg(MAX17040_VER_LSB);
> +
> + dev_info(&max17040->client->dev,
> + "MAX17040 Fuel-Gauge Ver %d%d\n", msb, lsb);
> +}
> +
> +static void max17040_get_online(void)
> +{
> + if (pdata->battery_online)
> + max17040->online = pdata->battery_online();
> + else
> + max17040->online = 1;
> +}
> +
> +static void max17040_get_status(void)
> +{
> + if (!pdata->charger_online || !pdata->charger_enable) {
> + max17040->status = POWER_SUPPLY_STATUS_UNKNOWN;
> + return;
> + }
> +
> + if (pdata->charger_online()) {
> + if (pdata->charger_enable())
> + max17040->status = POWER_SUPPLY_STATUS_CHARGING;
> + else
> + max17040->status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + } else {
> + max17040->status = POWER_SUPPLY_STATUS_DISCHARGING;
> + }
> +
> + if (max17040->soc > MAX17040_BATTERY_FULL)
> + max17040->status = POWER_SUPPLY_STATUS_FULL;
> +}
> +
> +static void max17040_work(struct work_struct *work)
> +{
> + max17040_get_vcell();
> + max17040_get_soc();
> + max17040_get_online();
> + max17040_get_status();
> +
> + schedule_delayed_work(&max17040->work, MAX17040_DELAY);
> +}
> +
> +static enum power_supply_property max17040_battery_props[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> + POWER_SUPPLY_PROP_CAPACITY,
> +};
> +
> +static int __devinit max17040_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct max17040_chip *chip;
> + int ret;
> +
> + chip = kzalloc(sizeof(struct max17040_chip), GFP_KERNEL);
> + if (!chip)
> + return -ENOMEM;
> +
> + chip->client = client;
> + pdata = client->dev.platform_data;
> +
> + i2c_set_clientdata(client, chip);
Please add i2c_check_functionality check before doing any smbus
read/write operations.
> + max17040 = chip;
This means that we support only one instance of this chip, right?
> +
> + max17040_reset();
> +
> + max17040_get_version();
> +
> + INIT_DELAYED_WORK_DEFERRABLE(&chip->work, max17040_work);
> + schedule_delayed_work(&chip->work, MAX17040_DELAY);
> +
> + bat_ps.properties = max17040_battery_props;
> + bat_ps.num_properties = ARRAY_SIZE(max17040_battery_props);
> +
> + ret = power_supply_register(&client->dev, &bat_ps);
> + if (ret) {
> + dev_err(&max17040->client->dev,
> + "failed: power supply register\n");
> + cancel_delayed_work(&chip->work);
> + i2c_set_clientdata(client, NULL);
> + kfree(chip);
> + max17040 = NULL;
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int __devexit max17040_remove(struct i2c_client *client)
> +{
> + struct max17040_chip *chip = i2c_get_clientdata(client);
> +
> + power_supply_unregister(&bat_ps);
> + cancel_delayed_work(&chip->work);
> + i2c_set_clientdata(client, NULL);
> + kfree(chip);
> + max17040 = NULL;
> + return 0;
> +}
> +
> +static int max17040_suspend(struct i2c_client *client,
> + pm_message_t state)
> +{
> + struct max17040_chip *chip = i2c_get_clientdata(client);
> +
> + cancel_delayed_work(&chip->work);
> + return 0;
> +}
> +
> +static int max17040_resume(struct i2c_client *client)
> +{
> + struct max17040_chip *chip = i2c_get_clientdata(client);
> +
> + schedule_delayed_work(&chip->work, MAX17040_DELAY);
> + return 0;
> +}
> +
> +static const struct i2c_device_id max17040_id[] = {
> + { "max17040", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, max17040_id);
> +
> +static struct i2c_driver max17040_i2c_driver = {
> + .driver = {
> + .name = "max17040",
> + },
> + .probe = max17040_probe,
> + .remove = __devexit_p(max17040_remove),
> + .suspend = max17040_suspend,
> + .resume = max17040_resume,
> + .id_table = max17040_id,
> +};
> +
> +static int __init max17040_init(void)
> +{
> + return i2c_add_driver(&max17040_i2c_driver);
> +}
> +module_init(max17040_init);
> +
> +static void __exit max17040_exit(void)
> +{
> + i2c_del_driver(&max17040_i2c_driver);
> +}
> +module_exit(max17040_exit);
> +
> +MODULE_AUTHOR("Minkyu Kang <mk7.kang@samsung.com>");
> +MODULE_DESCRIPTION("MAX17040 Fuel Gauge");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/max17040_battery.h b/include/linux/max17040_battery.h
> new file mode 100644
> index 0000000..ad97b06
> --- /dev/null
> +++ b/include/linux/max17040_battery.h
> @@ -0,0 +1,19 @@
> +/*
> + * Copyright (C) 2009 Samsung Electronics
> + * Minkyu Kang <mk7.kang@samsung.com>
> + *
> + * 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.
> + */
> +
> +#ifndef __MAX17040_BATTERY_H_
> +#define __MAX17040_BATTERY_H_
> +
> +struct max17040_platform_data {
> + int (*battery_online)(void);
> + int (*charger_online)(void);
> + int (*charger_enable)(void);
> +};
> +
> +#endif
> --
> 1.5.4.3
> --
> 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/
>
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
@ 2009-06-04 9:16 ` Trilok Soni
0 siblings, 0 replies; 18+ messages in thread
From: Trilok Soni @ 2009-06-04 9:16 UTC (permalink / raw)
To: Minkyu Kang
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, Anton Vorontsov,
Andrew Morton, linux-i2c-u79uwXL29TY76Z2rM5mHXA
Hi Minkyu Kang,
Adding linux-i2c mailing list, so not deleting any code.
On Thu, Jun 4, 2009 at 2:25 PM, Minkyu Kang <mk7.kang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> The MAX17040 is a I2C interfaced Fuel Gauge systems for lithium-ion batteries
> This patch adds support the MAX17040 Fuel Gauge
>
> Cc: Anton Vorontsov <cbou-JGs/UdohzUI@public.gmane.org>
> Signed-off-by: Minkyu Kang <mk7.kang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> drivers/power/Kconfig | 8 +
> drivers/power/Makefile | 3 +-
> drivers/power/max17040_battery.c | 306 ++++++++++++++++++++++++++++++++++++++
> include/linux/max17040_battery.h | 19 +++
> 4 files changed, 335 insertions(+), 1 deletions(-)
> create mode 100644 drivers/power/max17040_battery.c
> create mode 100644 include/linux/max17040_battery.h
>
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 33da112..6af5798 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -88,4 +88,12 @@ config CHARGER_PCF50633
> help
> Say Y to include support for NXP PCF50633 Main Battery Charger.
>
> +config BATTERY_MAX17040
> + tristate "Maxim MAX17040 Fuel Gauge"
> + depends on I2C
> + help
> + MAX17040 is fuel-gauge systems for lithium-ion (Li+) batteries
> + in handheld and portable equipment. The MAX17040 is configured
> + to operate with a single lithium cell
> +
> endif # POWER_SUPPLY
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index 2fcf41d..9c48995 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -25,4 +25,5 @@ obj-$(CONFIG_BATTERY_TOSA) += tosa_battery.o
> obj-$(CONFIG_BATTERY_WM97XX) += wm97xx_battery.o
> obj-$(CONFIG_BATTERY_BQ27x00) += bq27x00_battery.o
> obj-$(CONFIG_BATTERY_DA9030) += da9030_battery.o
> -obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
> \ No newline at end of file
> +obj-$(CONFIG_BATTERY_MAX17040)) += max17040_battery.o
> +obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
> diff --git a/drivers/power/max17040_battery.c b/drivers/power/max17040_battery.c
> new file mode 100644
> index 0000000..25303ba
> --- /dev/null
> +++ b/drivers/power/max17040_battery.c
> @@ -0,0 +1,306 @@
> +/*
> + * max17040_battery.c
> + * fuel-gauge systems for lithium-ion (Li+) batteries
> + *
> + * Copyright (C) 2009 Samsung Electronics
> + * Minkyu Kang <mk7.kang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * 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.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/mutex.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/power_supply.h>
> +#include <linux/max17040_battery.h>
> +
> +#define MAX17040_VCELL_MSB 0x02
> +#define MAX17040_VCELL_LSB 0x03
> +#define MAX17040_SOC_MSB 0x04
> +#define MAX17040_SOC_LSB 0x05
> +#define MAX17040_MODE_MSB 0x06
> +#define MAX17040_MODE_LSB 0x07
> +#define MAX17040_VER_MSB 0x08
> +#define MAX17040_VER_LSB 0x09
> +#define MAX17040_RCOMP_MSB 0x0C
> +#define MAX17040_RCOMP_LSB 0x0D
> +#define MAX17040_CMD_MSB 0xFE
> +#define MAX17040_CMD_LSB 0xFF
> +
> +#define MAX17040_DELAY 1000
> +#define MAX17040_BATTERY_FULL 95
> +
> +struct max17040_chip {
> + struct i2c_client *client;
> + struct delayed_work work;
> +
> + /* State Of Connect */
> + int online;
> + /* battery voltage */
> + int vcell;
> + /* battery capacity */
> + int soc;
> + /* State Of Charge */
> + int status;
> +};
> +
> +static struct max17040_chip *max17040;
> +static struct max17040_platform_data *pdata;
May be you want to move this pdata under chip structure.
> +
> +static int max17040_get_property(struct power_supply *bat_ps,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + switch (psp) {
> + case POWER_SUPPLY_PROP_STATUS:
> + val->intval = max17040->status;
> + break;
> + case POWER_SUPPLY_PROP_ONLINE:
> + val->intval = max17040->online;
> + break;
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + val->intval = max17040->vcell;
> + break;
> + case POWER_SUPPLY_PROP_CAPACITY:
> + val->intval = max17040->soc;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static struct power_supply bat_ps = {
> + .name = "battery",
> + .type = POWER_SUPPLY_TYPE_BATTERY,
> + .get_property = max17040_get_property,
> +};
> +
> +static int max17040_write_reg(int reg, u8 value)
> +{
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(max17040->client, reg, value);
> +
> + if (ret < 0) {
> + dev_err(&max17040->client->dev,
> + "%s: reg 0x%x, val 0x%x, err %d\n",
> + __func__, reg, value, ret);
> + }
> +
> + return ret;
> +}
> +
> +static u8 max17040_read_reg(int reg)
> +{
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(max17040->client, reg);
> +
> + if (ret < 0) {
> + dev_err(&max17040->client->dev,
> + "%s: reg 0x%x, err %d\n",
> + __func__, reg, ret);
> + }
> +
> + return ret;
> +}
> +
> +static void max17040_reset(void)
> +{
> + max17040_write_reg(MAX17040_CMD_MSB, 0x54);
> + max17040_write_reg(MAX17040_CMD_LSB, 0x00);
> +}
> +
> +static void max17040_set_rcomp(u16 val)
> +{
> + max17040_write_reg(MAX17040_RCOMP_MSB, val >> 8);
> + max17040_write_reg(MAX17040_RCOMP_LSB, (val << 8) >> 8);
> +}
> +
> +static void max17040_get_vcell(void)
> +{
> + u8 msb;
> + u8 lsb;
> +
> + msb = max17040_read_reg(MAX17040_VCELL_MSB);
> + lsb = max17040_read_reg(MAX17040_VCELL_LSB);
> +
> + max17040->vcell = (msb << 4) + (lsb >> 4);
> +}
> +
> +static void max17040_get_soc(void)
> +{
> + u8 msb;
> + u8 lsb;
> +
> + msb = max17040_read_reg(MAX17040_SOC_MSB);
> + lsb = max17040_read_reg(MAX17040_SOC_LSB);
> +
> + max17040->soc = msb;
> +}
> +
> +static void max17040_get_version(void)
> +{
> + u8 msb;
> + u8 lsb;
> +
> + msb = max17040_read_reg(MAX17040_VER_MSB);
> + lsb = max17040_read_reg(MAX17040_VER_LSB);
> +
> + dev_info(&max17040->client->dev,
> + "MAX17040 Fuel-Gauge Ver %d%d\n", msb, lsb);
> +}
> +
> +static void max17040_get_online(void)
> +{
> + if (pdata->battery_online)
> + max17040->online = pdata->battery_online();
> + else
> + max17040->online = 1;
> +}
> +
> +static void max17040_get_status(void)
> +{
> + if (!pdata->charger_online || !pdata->charger_enable) {
> + max17040->status = POWER_SUPPLY_STATUS_UNKNOWN;
> + return;
> + }
> +
> + if (pdata->charger_online()) {
> + if (pdata->charger_enable())
> + max17040->status = POWER_SUPPLY_STATUS_CHARGING;
> + else
> + max17040->status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + } else {
> + max17040->status = POWER_SUPPLY_STATUS_DISCHARGING;
> + }
> +
> + if (max17040->soc > MAX17040_BATTERY_FULL)
> + max17040->status = POWER_SUPPLY_STATUS_FULL;
> +}
> +
> +static void max17040_work(struct work_struct *work)
> +{
> + max17040_get_vcell();
> + max17040_get_soc();
> + max17040_get_online();
> + max17040_get_status();
> +
> + schedule_delayed_work(&max17040->work, MAX17040_DELAY);
> +}
> +
> +static enum power_supply_property max17040_battery_props[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> + POWER_SUPPLY_PROP_CAPACITY,
> +};
> +
> +static int __devinit max17040_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct max17040_chip *chip;
> + int ret;
> +
> + chip = kzalloc(sizeof(struct max17040_chip), GFP_KERNEL);
> + if (!chip)
> + return -ENOMEM;
> +
> + chip->client = client;
> + pdata = client->dev.platform_data;
> +
> + i2c_set_clientdata(client, chip);
Please add i2c_check_functionality check before doing any smbus
read/write operations.
> + max17040 = chip;
This means that we support only one instance of this chip, right?
> +
> + max17040_reset();
> +
> + max17040_get_version();
> +
> + INIT_DELAYED_WORK_DEFERRABLE(&chip->work, max17040_work);
> + schedule_delayed_work(&chip->work, MAX17040_DELAY);
> +
> + bat_ps.properties = max17040_battery_props;
> + bat_ps.num_properties = ARRAY_SIZE(max17040_battery_props);
> +
> + ret = power_supply_register(&client->dev, &bat_ps);
> + if (ret) {
> + dev_err(&max17040->client->dev,
> + "failed: power supply register\n");
> + cancel_delayed_work(&chip->work);
> + i2c_set_clientdata(client, NULL);
> + kfree(chip);
> + max17040 = NULL;
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int __devexit max17040_remove(struct i2c_client *client)
> +{
> + struct max17040_chip *chip = i2c_get_clientdata(client);
> +
> + power_supply_unregister(&bat_ps);
> + cancel_delayed_work(&chip->work);
> + i2c_set_clientdata(client, NULL);
> + kfree(chip);
> + max17040 = NULL;
> + return 0;
> +}
> +
> +static int max17040_suspend(struct i2c_client *client,
> + pm_message_t state)
> +{
> + struct max17040_chip *chip = i2c_get_clientdata(client);
> +
> + cancel_delayed_work(&chip->work);
> + return 0;
> +}
> +
> +static int max17040_resume(struct i2c_client *client)
> +{
> + struct max17040_chip *chip = i2c_get_clientdata(client);
> +
> + schedule_delayed_work(&chip->work, MAX17040_DELAY);
> + return 0;
> +}
> +
> +static const struct i2c_device_id max17040_id[] = {
> + { "max17040", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, max17040_id);
> +
> +static struct i2c_driver max17040_i2c_driver = {
> + .driver = {
> + .name = "max17040",
> + },
> + .probe = max17040_probe,
> + .remove = __devexit_p(max17040_remove),
> + .suspend = max17040_suspend,
> + .resume = max17040_resume,
> + .id_table = max17040_id,
> +};
> +
> +static int __init max17040_init(void)
> +{
> + return i2c_add_driver(&max17040_i2c_driver);
> +}
> +module_init(max17040_init);
> +
> +static void __exit max17040_exit(void)
> +{
> + i2c_del_driver(&max17040_i2c_driver);
> +}
> +module_exit(max17040_exit);
> +
> +MODULE_AUTHOR("Minkyu Kang <mk7.kang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>");
> +MODULE_DESCRIPTION("MAX17040 Fuel Gauge");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/max17040_battery.h b/include/linux/max17040_battery.h
> new file mode 100644
> index 0000000..ad97b06
> --- /dev/null
> +++ b/include/linux/max17040_battery.h
> @@ -0,0 +1,19 @@
> +/*
> + * Copyright (C) 2009 Samsung Electronics
> + * Minkyu Kang <mk7.kang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * 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.
> + */
> +
> +#ifndef __MAX17040_BATTERY_H_
> +#define __MAX17040_BATTERY_H_
> +
> +struct max17040_platform_data {
> + int (*battery_online)(void);
> + int (*charger_online)(void);
> + int (*charger_enable)(void);
> +};
> +
> +#endif
> --
> 1.5.4.3
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
2009-06-04 8:55 ` Minkyu Kang
` (3 preceding siblings ...)
(?)
@ 2009-06-04 9:16 ` Trilok Soni
-1 siblings, 0 replies; 18+ messages in thread
From: Trilok Soni @ 2009-06-04 9:16 UTC (permalink / raw)
To: Minkyu Kang
Cc: linux-kernel, Anton Vorontsov, kyungmin.park, linux-i2c,
linux-pm, Andrew Morton
Hi Minkyu Kang,
Adding linux-i2c mailing list, so not deleting any code.
On Thu, Jun 4, 2009 at 2:25 PM, Minkyu Kang <mk7.kang@samsung.com> wrote:
> The MAX17040 is a I2C interfaced Fuel Gauge systems for lithium-ion batteries
> This patch adds support the MAX17040 Fuel Gauge
>
> Cc: Anton Vorontsov <cbou@mail.ru>
> Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
> ---
> drivers/power/Kconfig | 8 +
> drivers/power/Makefile | 3 +-
> drivers/power/max17040_battery.c | 306 ++++++++++++++++++++++++++++++++++++++
> include/linux/max17040_battery.h | 19 +++
> 4 files changed, 335 insertions(+), 1 deletions(-)
> create mode 100644 drivers/power/max17040_battery.c
> create mode 100644 include/linux/max17040_battery.h
>
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 33da112..6af5798 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -88,4 +88,12 @@ config CHARGER_PCF50633
> help
> Say Y to include support for NXP PCF50633 Main Battery Charger.
>
> +config BATTERY_MAX17040
> + tristate "Maxim MAX17040 Fuel Gauge"
> + depends on I2C
> + help
> + MAX17040 is fuel-gauge systems for lithium-ion (Li+) batteries
> + in handheld and portable equipment. The MAX17040 is configured
> + to operate with a single lithium cell
> +
> endif # POWER_SUPPLY
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index 2fcf41d..9c48995 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -25,4 +25,5 @@ obj-$(CONFIG_BATTERY_TOSA) += tosa_battery.o
> obj-$(CONFIG_BATTERY_WM97XX) += wm97xx_battery.o
> obj-$(CONFIG_BATTERY_BQ27x00) += bq27x00_battery.o
> obj-$(CONFIG_BATTERY_DA9030) += da9030_battery.o
> -obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
> \ No newline at end of file
> +obj-$(CONFIG_BATTERY_MAX17040)) += max17040_battery.o
> +obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
> diff --git a/drivers/power/max17040_battery.c b/drivers/power/max17040_battery.c
> new file mode 100644
> index 0000000..25303ba
> --- /dev/null
> +++ b/drivers/power/max17040_battery.c
> @@ -0,0 +1,306 @@
> +/*
> + * max17040_battery.c
> + * fuel-gauge systems for lithium-ion (Li+) batteries
> + *
> + * Copyright (C) 2009 Samsung Electronics
> + * Minkyu Kang <mk7.kang@samsung.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/mutex.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/power_supply.h>
> +#include <linux/max17040_battery.h>
> +
> +#define MAX17040_VCELL_MSB 0x02
> +#define MAX17040_VCELL_LSB 0x03
> +#define MAX17040_SOC_MSB 0x04
> +#define MAX17040_SOC_LSB 0x05
> +#define MAX17040_MODE_MSB 0x06
> +#define MAX17040_MODE_LSB 0x07
> +#define MAX17040_VER_MSB 0x08
> +#define MAX17040_VER_LSB 0x09
> +#define MAX17040_RCOMP_MSB 0x0C
> +#define MAX17040_RCOMP_LSB 0x0D
> +#define MAX17040_CMD_MSB 0xFE
> +#define MAX17040_CMD_LSB 0xFF
> +
> +#define MAX17040_DELAY 1000
> +#define MAX17040_BATTERY_FULL 95
> +
> +struct max17040_chip {
> + struct i2c_client *client;
> + struct delayed_work work;
> +
> + /* State Of Connect */
> + int online;
> + /* battery voltage */
> + int vcell;
> + /* battery capacity */
> + int soc;
> + /* State Of Charge */
> + int status;
> +};
> +
> +static struct max17040_chip *max17040;
> +static struct max17040_platform_data *pdata;
May be you want to move this pdata under chip structure.
> +
> +static int max17040_get_property(struct power_supply *bat_ps,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + switch (psp) {
> + case POWER_SUPPLY_PROP_STATUS:
> + val->intval = max17040->status;
> + break;
> + case POWER_SUPPLY_PROP_ONLINE:
> + val->intval = max17040->online;
> + break;
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + val->intval = max17040->vcell;
> + break;
> + case POWER_SUPPLY_PROP_CAPACITY:
> + val->intval = max17040->soc;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static struct power_supply bat_ps = {
> + .name = "battery",
> + .type = POWER_SUPPLY_TYPE_BATTERY,
> + .get_property = max17040_get_property,
> +};
> +
> +static int max17040_write_reg(int reg, u8 value)
> +{
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(max17040->client, reg, value);
> +
> + if (ret < 0) {
> + dev_err(&max17040->client->dev,
> + "%s: reg 0x%x, val 0x%x, err %d\n",
> + __func__, reg, value, ret);
> + }
> +
> + return ret;
> +}
> +
> +static u8 max17040_read_reg(int reg)
> +{
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(max17040->client, reg);
> +
> + if (ret < 0) {
> + dev_err(&max17040->client->dev,
> + "%s: reg 0x%x, err %d\n",
> + __func__, reg, ret);
> + }
> +
> + return ret;
> +}
> +
> +static void max17040_reset(void)
> +{
> + max17040_write_reg(MAX17040_CMD_MSB, 0x54);
> + max17040_write_reg(MAX17040_CMD_LSB, 0x00);
> +}
> +
> +static void max17040_set_rcomp(u16 val)
> +{
> + max17040_write_reg(MAX17040_RCOMP_MSB, val >> 8);
> + max17040_write_reg(MAX17040_RCOMP_LSB, (val << 8) >> 8);
> +}
> +
> +static void max17040_get_vcell(void)
> +{
> + u8 msb;
> + u8 lsb;
> +
> + msb = max17040_read_reg(MAX17040_VCELL_MSB);
> + lsb = max17040_read_reg(MAX17040_VCELL_LSB);
> +
> + max17040->vcell = (msb << 4) + (lsb >> 4);
> +}
> +
> +static void max17040_get_soc(void)
> +{
> + u8 msb;
> + u8 lsb;
> +
> + msb = max17040_read_reg(MAX17040_SOC_MSB);
> + lsb = max17040_read_reg(MAX17040_SOC_LSB);
> +
> + max17040->soc = msb;
> +}
> +
> +static void max17040_get_version(void)
> +{
> + u8 msb;
> + u8 lsb;
> +
> + msb = max17040_read_reg(MAX17040_VER_MSB);
> + lsb = max17040_read_reg(MAX17040_VER_LSB);
> +
> + dev_info(&max17040->client->dev,
> + "MAX17040 Fuel-Gauge Ver %d%d\n", msb, lsb);
> +}
> +
> +static void max17040_get_online(void)
> +{
> + if (pdata->battery_online)
> + max17040->online = pdata->battery_online();
> + else
> + max17040->online = 1;
> +}
> +
> +static void max17040_get_status(void)
> +{
> + if (!pdata->charger_online || !pdata->charger_enable) {
> + max17040->status = POWER_SUPPLY_STATUS_UNKNOWN;
> + return;
> + }
> +
> + if (pdata->charger_online()) {
> + if (pdata->charger_enable())
> + max17040->status = POWER_SUPPLY_STATUS_CHARGING;
> + else
> + max17040->status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + } else {
> + max17040->status = POWER_SUPPLY_STATUS_DISCHARGING;
> + }
> +
> + if (max17040->soc > MAX17040_BATTERY_FULL)
> + max17040->status = POWER_SUPPLY_STATUS_FULL;
> +}
> +
> +static void max17040_work(struct work_struct *work)
> +{
> + max17040_get_vcell();
> + max17040_get_soc();
> + max17040_get_online();
> + max17040_get_status();
> +
> + schedule_delayed_work(&max17040->work, MAX17040_DELAY);
> +}
> +
> +static enum power_supply_property max17040_battery_props[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> + POWER_SUPPLY_PROP_CAPACITY,
> +};
> +
> +static int __devinit max17040_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct max17040_chip *chip;
> + int ret;
> +
> + chip = kzalloc(sizeof(struct max17040_chip), GFP_KERNEL);
> + if (!chip)
> + return -ENOMEM;
> +
> + chip->client = client;
> + pdata = client->dev.platform_data;
> +
> + i2c_set_clientdata(client, chip);
Please add i2c_check_functionality check before doing any smbus
read/write operations.
> + max17040 = chip;
This means that we support only one instance of this chip, right?
> +
> + max17040_reset();
> +
> + max17040_get_version();
> +
> + INIT_DELAYED_WORK_DEFERRABLE(&chip->work, max17040_work);
> + schedule_delayed_work(&chip->work, MAX17040_DELAY);
> +
> + bat_ps.properties = max17040_battery_props;
> + bat_ps.num_properties = ARRAY_SIZE(max17040_battery_props);
> +
> + ret = power_supply_register(&client->dev, &bat_ps);
> + if (ret) {
> + dev_err(&max17040->client->dev,
> + "failed: power supply register\n");
> + cancel_delayed_work(&chip->work);
> + i2c_set_clientdata(client, NULL);
> + kfree(chip);
> + max17040 = NULL;
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int __devexit max17040_remove(struct i2c_client *client)
> +{
> + struct max17040_chip *chip = i2c_get_clientdata(client);
> +
> + power_supply_unregister(&bat_ps);
> + cancel_delayed_work(&chip->work);
> + i2c_set_clientdata(client, NULL);
> + kfree(chip);
> + max17040 = NULL;
> + return 0;
> +}
> +
> +static int max17040_suspend(struct i2c_client *client,
> + pm_message_t state)
> +{
> + struct max17040_chip *chip = i2c_get_clientdata(client);
> +
> + cancel_delayed_work(&chip->work);
> + return 0;
> +}
> +
> +static int max17040_resume(struct i2c_client *client)
> +{
> + struct max17040_chip *chip = i2c_get_clientdata(client);
> +
> + schedule_delayed_work(&chip->work, MAX17040_DELAY);
> + return 0;
> +}
> +
> +static const struct i2c_device_id max17040_id[] = {
> + { "max17040", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, max17040_id);
> +
> +static struct i2c_driver max17040_i2c_driver = {
> + .driver = {
> + .name = "max17040",
> + },
> + .probe = max17040_probe,
> + .remove = __devexit_p(max17040_remove),
> + .suspend = max17040_suspend,
> + .resume = max17040_resume,
> + .id_table = max17040_id,
> +};
> +
> +static int __init max17040_init(void)
> +{
> + return i2c_add_driver(&max17040_i2c_driver);
> +}
> +module_init(max17040_init);
> +
> +static void __exit max17040_exit(void)
> +{
> + i2c_del_driver(&max17040_i2c_driver);
> +}
> +module_exit(max17040_exit);
> +
> +MODULE_AUTHOR("Minkyu Kang <mk7.kang@samsung.com>");
> +MODULE_DESCRIPTION("MAX17040 Fuel Gauge");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/max17040_battery.h b/include/linux/max17040_battery.h
> new file mode 100644
> index 0000000..ad97b06
> --- /dev/null
> +++ b/include/linux/max17040_battery.h
> @@ -0,0 +1,19 @@
> +/*
> + * Copyright (C) 2009 Samsung Electronics
> + * Minkyu Kang <mk7.kang@samsung.com>
> + *
> + * 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.
> + */
> +
> +#ifndef __MAX17040_BATTERY_H_
> +#define __MAX17040_BATTERY_H_
> +
> +struct max17040_platform_data {
> + int (*battery_online)(void);
> + int (*charger_online)(void);
> + int (*charger_enable)(void);
> +};
> +
> +#endif
> --
> 1.5.4.3
> --
> 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/
>
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
@ 2009-06-04 9:35 ` Jean Delvare
0 siblings, 0 replies; 18+ messages in thread
From: Jean Delvare @ 2009-06-04 9:35 UTC (permalink / raw)
To: Minkyu Kang
Cc: Trilok Soni, linux-kernel, linux-pm, kyungmin.park,
Anton Vorontsov, Andrew Morton, linux-i2c
On Thu, 4 Jun 2009 14:46:45 +0530, Trilok Soni wrote:
> Hi Minkyu Kang,
>
> Adding linux-i2c mailing list, so not deleting any code.
>
> On Thu, Jun 4, 2009 at 2:25 PM, Minkyu Kang <mk7.kang@samsung.com> wrote:
> > The MAX17040 is a I2C interfaced Fuel Gauge systems for lithium-ion batteries
> > This patch adds support the MAX17040 Fuel Gauge
> >
> > Cc: Anton Vorontsov <cbou@mail.ru>
> > Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
> > ---
> > drivers/power/Kconfig | 8 +
> > drivers/power/Makefile | 3 +-
> > drivers/power/max17040_battery.c | 306 ++++++++++++++++++++++++++++++++++++++
> > include/linux/max17040_battery.h | 19 +++
> > 4 files changed, 335 insertions(+), 1 deletions(-)
> > create mode 100644 drivers/power/max17040_battery.c
> > create mode 100644 include/linux/max17040_battery.h
> > (...)
> > +static u8 max17040_read_reg(int reg)
> > +{
> > + int ret;
> > +
> > + ret = i2c_smbus_read_byte_data(max17040->client, reg);
> > +
> > + if (ret < 0) {
> > + dev_err(&max17040->client->dev,
> > + "%s: reg 0x%x, err %d\n",
> > + __func__, reg, ret);
> > + }
> > +
> > + return ret;
> > +}
In case of error, this will wrap the error code into a u8 and the
caller won't notice. So you'll return a random value, depending on the
actual error which happened. No good.
You should either return an int there, and have the caller check for
errors, or if you don't want to care about errors, return an arbitrary
value on error (e.g. 0.)
> > (...)
> > +static int __devinit max17040_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct max17040_chip *chip;
> > + int ret;
> > +
> > + chip = kzalloc(sizeof(struct max17040_chip), GFP_KERNEL);
> > + if (!chip)
> > + return -ENOMEM;
> > +
> > + chip->client = client;
> > + pdata = client->dev.platform_data;
> > +
> > + i2c_set_clientdata(client, chip);
>
> Please add i2c_check_functionality check before doing any smbus
> read/write operations.
>
> > + max17040 = chip;
>
> This means that we support only one instance of this chip, right?
>
> > +
> > + max17040_reset();
> > +
> > + max17040_get_version();
> > +
> > + INIT_DELAYED_WORK_DEFERRABLE(&chip->work, max17040_work);
> > + schedule_delayed_work(&chip->work, MAX17040_DELAY);
> > +
> > + bat_ps.properties = max17040_battery_props;
> > + bat_ps.num_properties = ARRAY_SIZE(max17040_battery_props);
> > +
> > + ret = power_supply_register(&client->dev, &bat_ps);
> > + if (ret) {
> > + dev_err(&max17040->client->dev,
> > + "failed: power supply register\n");
> > + cancel_delayed_work(&chip->work);
> > + i2c_set_clientdata(client, NULL);
> > + kfree(chip);
> > + max17040 = NULL;
> > + return -1;
Please come up with a better error code.
> > + }
> > +
> > + return 0;
> > +}
--
Jean Delvare
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
@ 2009-06-04 9:35 ` Jean Delvare
0 siblings, 0 replies; 18+ messages in thread
From: Jean Delvare @ 2009-06-04 9:35 UTC (permalink / raw)
To: Minkyu Kang
Cc: Trilok Soni, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, Anton Vorontsov,
Andrew Morton, linux-i2c-u79uwXL29TY76Z2rM5mHXA
On Thu, 4 Jun 2009 14:46:45 +0530, Trilok Soni wrote:
> Hi Minkyu Kang,
>
> Adding linux-i2c mailing list, so not deleting any code.
>
> On Thu, Jun 4, 2009 at 2:25 PM, Minkyu Kang <mk7.kang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> > The MAX17040 is a I2C interfaced Fuel Gauge systems for lithium-ion batteries
> > This patch adds support the MAX17040 Fuel Gauge
> >
> > Cc: Anton Vorontsov <cbou-JGs/UdohzUI@public.gmane.org>
> > Signed-off-by: Minkyu Kang <mk7.kang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > ---
> > drivers/power/Kconfig | 8 +
> > drivers/power/Makefile | 3 +-
> > drivers/power/max17040_battery.c | 306 ++++++++++++++++++++++++++++++++++++++
> > include/linux/max17040_battery.h | 19 +++
> > 4 files changed, 335 insertions(+), 1 deletions(-)
> > create mode 100644 drivers/power/max17040_battery.c
> > create mode 100644 include/linux/max17040_battery.h
> > (...)
> > +static u8 max17040_read_reg(int reg)
> > +{
> > + int ret;
> > +
> > + ret = i2c_smbus_read_byte_data(max17040->client, reg);
> > +
> > + if (ret < 0) {
> > + dev_err(&max17040->client->dev,
> > + "%s: reg 0x%x, err %d\n",
> > + __func__, reg, ret);
> > + }
> > +
> > + return ret;
> > +}
In case of error, this will wrap the error code into a u8 and the
caller won't notice. So you'll return a random value, depending on the
actual error which happened. No good.
You should either return an int there, and have the caller check for
errors, or if you don't want to care about errors, return an arbitrary
value on error (e.g. 0.)
> > (...)
> > +static int __devinit max17040_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct max17040_chip *chip;
> > + int ret;
> > +
> > + chip = kzalloc(sizeof(struct max17040_chip), GFP_KERNEL);
> > + if (!chip)
> > + return -ENOMEM;
> > +
> > + chip->client = client;
> > + pdata = client->dev.platform_data;
> > +
> > + i2c_set_clientdata(client, chip);
>
> Please add i2c_check_functionality check before doing any smbus
> read/write operations.
>
> > + max17040 = chip;
>
> This means that we support only one instance of this chip, right?
>
> > +
> > + max17040_reset();
> > +
> > + max17040_get_version();
> > +
> > + INIT_DELAYED_WORK_DEFERRABLE(&chip->work, max17040_work);
> > + schedule_delayed_work(&chip->work, MAX17040_DELAY);
> > +
> > + bat_ps.properties = max17040_battery_props;
> > + bat_ps.num_properties = ARRAY_SIZE(max17040_battery_props);
> > +
> > + ret = power_supply_register(&client->dev, &bat_ps);
> > + if (ret) {
> > + dev_err(&max17040->client->dev,
> > + "failed: power supply register\n");
> > + cancel_delayed_work(&chip->work);
> > + i2c_set_clientdata(client, NULL);
> > + kfree(chip);
> > + max17040 = NULL;
> > + return -1;
Please come up with a better error code.
> > + }
> > +
> > + return 0;
> > +}
--
Jean Delvare
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
2009-06-04 9:16 ` Trilok Soni
(?)
(?)
@ 2009-06-04 9:35 ` Jean Delvare
-1 siblings, 0 replies; 18+ messages in thread
From: Jean Delvare @ 2009-06-04 9:35 UTC (permalink / raw)
To: Minkyu Kang
Cc: Trilok Soni, Anton, linux-kernel, Vorontsov, kyungmin.park,
linux-i2c, linux-pm, Andrew Morton
On Thu, 4 Jun 2009 14:46:45 +0530, Trilok Soni wrote:
> Hi Minkyu Kang,
>
> Adding linux-i2c mailing list, so not deleting any code.
>
> On Thu, Jun 4, 2009 at 2:25 PM, Minkyu Kang <mk7.kang@samsung.com> wrote:
> > The MAX17040 is a I2C interfaced Fuel Gauge systems for lithium-ion batteries
> > This patch adds support the MAX17040 Fuel Gauge
> >
> > Cc: Anton Vorontsov <cbou@mail.ru>
> > Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
> > ---
> > drivers/power/Kconfig | 8 +
> > drivers/power/Makefile | 3 +-
> > drivers/power/max17040_battery.c | 306 ++++++++++++++++++++++++++++++++++++++
> > include/linux/max17040_battery.h | 19 +++
> > 4 files changed, 335 insertions(+), 1 deletions(-)
> > create mode 100644 drivers/power/max17040_battery.c
> > create mode 100644 include/linux/max17040_battery.h
> > (...)
> > +static u8 max17040_read_reg(int reg)
> > +{
> > + int ret;
> > +
> > + ret = i2c_smbus_read_byte_data(max17040->client, reg);
> > +
> > + if (ret < 0) {
> > + dev_err(&max17040->client->dev,
> > + "%s: reg 0x%x, err %d\n",
> > + __func__, reg, ret);
> > + }
> > +
> > + return ret;
> > +}
In case of error, this will wrap the error code into a u8 and the
caller won't notice. So you'll return a random value, depending on the
actual error which happened. No good.
You should either return an int there, and have the caller check for
errors, or if you don't want to care about errors, return an arbitrary
value on error (e.g. 0.)
> > (...)
> > +static int __devinit max17040_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct max17040_chip *chip;
> > + int ret;
> > +
> > + chip = kzalloc(sizeof(struct max17040_chip), GFP_KERNEL);
> > + if (!chip)
> > + return -ENOMEM;
> > +
> > + chip->client = client;
> > + pdata = client->dev.platform_data;
> > +
> > + i2c_set_clientdata(client, chip);
>
> Please add i2c_check_functionality check before doing any smbus
> read/write operations.
>
> > + max17040 = chip;
>
> This means that we support only one instance of this chip, right?
>
> > +
> > + max17040_reset();
> > +
> > + max17040_get_version();
> > +
> > + INIT_DELAYED_WORK_DEFERRABLE(&chip->work, max17040_work);
> > + schedule_delayed_work(&chip->work, MAX17040_DELAY);
> > +
> > + bat_ps.properties = max17040_battery_props;
> > + bat_ps.num_properties = ARRAY_SIZE(max17040_battery_props);
> > +
> > + ret = power_supply_register(&client->dev, &bat_ps);
> > + if (ret) {
> > + dev_err(&max17040->client->dev,
> > + "failed: power supply register\n");
> > + cancel_delayed_work(&chip->work);
> > + i2c_set_clientdata(client, NULL);
> > + kfree(chip);
> > + max17040 = NULL;
> > + return -1;
Please come up with a better error code.
> > + }
> > +
> > + return 0;
> > +}
--
Jean Delvare
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
2009-06-04 9:06 ` Andrew Morton
@ 2009-06-04 10:39 ` Minkyu Kang
2009-06-04 10:39 ` Minkyu Kang
1 sibling, 0 replies; 18+ messages in thread
From: Minkyu Kang @ 2009-06-04 10:39 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, linux-pm
> Wouldn't it be better to start the delayed_work after the
> power_supply_register() has succeeded?
>
Yes, that's better. thanks :)
--
from. prom.
promsoft.net
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
2009-06-04 9:06 ` Andrew Morton
2009-06-04 10:39 ` Minkyu Kang
@ 2009-06-04 10:39 ` Minkyu Kang
1 sibling, 0 replies; 18+ messages in thread
From: Minkyu Kang @ 2009-06-04 10:39 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-pm, linux-kernel
> Wouldn't it be better to start the delayed_work after the
> power_supply_register() has succeeded?
>
Yes, that's better. thanks :)
--
from. prom.
promsoft.net
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
@ 2009-06-04 10:47 ` Minkyu Kang
0 siblings, 0 replies; 18+ messages in thread
From: Minkyu Kang @ 2009-06-04 10:47 UTC (permalink / raw)
To: Trilok Soni; +Cc: Minkyu Kang, linux-kernel, linux-pm, linux-i2c
Hi, Trilok
> Adding linux-i2c mailing list, so not deleting any code.
Ok, I will.
>> +static struct max17040_chip *max17040;
>> +static struct max17040_platform_data *pdata;
>
> May be you want to move this pdata under chip structure.
>
>> + i2c_set_clientdata(client, chip);
>
> Please add i2c_check_functionality check before doing any smbus
> read/write operations.
>
Ok, that's better.
>> + max17040 = chip;
>
> This means that we support only one instance of this chip, right?
>
Yes right.. but, I think that is not a good way.
I'll modify it, too. thanks :)
--
from. prom.
promsoft.net
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
@ 2009-06-04 10:47 ` Minkyu Kang
0 siblings, 0 replies; 18+ messages in thread
From: Minkyu Kang @ 2009-06-04 10:47 UTC (permalink / raw)
To: Trilok Soni
Cc: Minkyu Kang, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
Hi, Trilok
> Adding linux-i2c mailing list, so not deleting any code.
Ok, I will.
>> +static struct max17040_chip *max17040;
>> +static struct max17040_platform_data *pdata;
>
> May be you want to move this pdata under chip structure.
>
>> + i2c_set_clientdata(client, chip);
>
> Please add i2c_check_functionality check before doing any smbus
> read/write operations.
>
Ok, that's better.
>> + max17040 = chip;
>
> This means that we support only one instance of this chip, right?
>
Yes right.. but, I think that is not a good way.
I'll modify it, too. thanks :)
--
from. prom.
promsoft.net
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
2009-06-04 9:16 ` Trilok Soni
` (3 preceding siblings ...)
(?)
@ 2009-06-04 10:47 ` Minkyu Kang
-1 siblings, 0 replies; 18+ messages in thread
From: Minkyu Kang @ 2009-06-04 10:47 UTC (permalink / raw)
To: Trilok Soni; +Cc: linux-pm, Minkyu Kang, linux-kernel, linux-i2c
Hi, Trilok
> Adding linux-i2c mailing list, so not deleting any code.
Ok, I will.
>> +static struct max17040_chip *max17040;
>> +static struct max17040_platform_data *pdata;
>
> May be you want to move this pdata under chip structure.
>
>> + i2c_set_clientdata(client, chip);
>
> Please add i2c_check_functionality check before doing any smbus
> read/write operations.
>
Ok, that's better.
>> + max17040 = chip;
>
> This means that we support only one instance of this chip, right?
>
Yes right.. but, I think that is not a good way.
I'll modify it, too. thanks :)
--
from. prom.
promsoft.net
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
@ 2009-06-04 10:55 ` Minkyu Kang
0 siblings, 0 replies; 18+ messages in thread
From: Minkyu Kang @ 2009-06-04 10:55 UTC (permalink / raw)
To: Jean Delvare
Cc: Minkyu Kang, linux-kernel, linux-pm, Anton Vorontsov, linux-i2c
Dear, Jean Delvare
>
> In case of error, this will wrap the error code into a u8 and the
> caller won't notice. So you'll return a random value, depending on the
> actual error which happened. No good.
>
> You should either return an int there, and have the caller check for
> errors, or if you don't want to care about errors, return an arbitrary
> value on error (e.g. 0.)
>
Yes, I missed it.
>> > + ret = power_supply_register(&client->dev, &bat_ps);
>> > + if (ret) {
>> > + dev_err(&max17040->client->dev,
>> > + "failed: power supply register\n");
>> > + cancel_delayed_work(&chip->work);
>> > + i2c_set_clientdata(client, NULL);
>> > + kfree(chip);
>> > + max17040 = NULL;
>> > + return -1;
>
> Please come up with a better error code.
>
Ok, many thanks :)
--
from. prom.
promsoft.net
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
@ 2009-06-04 10:55 ` Minkyu Kang
0 siblings, 0 replies; 18+ messages in thread
From: Minkyu Kang @ 2009-06-04 10:55 UTC (permalink / raw)
To: Jean Delvare
Cc: Minkyu Kang, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Anton Vorontsov, linux-i2c-u79uwXL29TY76Z2rM5mHXA
Dear, Jean Delvare
>
> In case of error, this will wrap the error code into a u8 and the
> caller won't notice. So you'll return a random value, depending on the
> actual error which happened. No good.
>
> You should either return an int there, and have the caller check for
> errors, or if you don't want to care about errors, return an arbitrary
> value on error (e.g. 0.)
>
Yes, I missed it.
>> > + ret = power_supply_register(&client->dev, &bat_ps);
>> > + if (ret) {
>> > + dev_err(&max17040->client->dev,
>> > + "failed: power supply register\n");
>> > + cancel_delayed_work(&chip->work);
>> > + i2c_set_clientdata(client, NULL);
>> > + kfree(chip);
>> > + max17040 = NULL;
>> > + return -1;
>
> Please come up with a better error code.
>
Ok, many thanks :)
--
from. prom.
promsoft.net
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] add MAX17040 Fuel Gauge driver
2009-06-04 9:35 ` Jean Delvare
(?)
@ 2009-06-04 10:55 ` Minkyu Kang
-1 siblings, 0 replies; 18+ messages in thread
From: Minkyu Kang @ 2009-06-04 10:55 UTC (permalink / raw)
To: Jean Delvare
Cc: Anton Vorontsov, linux-pm, Minkyu Kang, linux-kernel, linux-i2c
Dear, Jean Delvare
>
> In case of error, this will wrap the error code into a u8 and the
> caller won't notice. So you'll return a random value, depending on the
> actual error which happened. No good.
>
> You should either return an int there, and have the caller check for
> errors, or if you don't want to care about errors, return an arbitrary
> value on error (e.g. 0.)
>
Yes, I missed it.
>> > + ret = power_supply_register(&client->dev, &bat_ps);
>> > + if (ret) {
>> > + dev_err(&max17040->client->dev,
>> > + "failed: power supply register\n");
>> > + cancel_delayed_work(&chip->work);
>> > + i2c_set_clientdata(client, NULL);
>> > + kfree(chip);
>> > + max17040 = NULL;
>> > + return -1;
>
> Please come up with a better error code.
>
Ok, many thanks :)
--
from. prom.
promsoft.net
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2009-06-04 10:55 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-04 8:55 [PATCH v2] add MAX17040 Fuel Gauge driver Minkyu Kang
2009-06-04 8:55 ` Minkyu Kang
2009-06-04 9:06 ` Andrew Morton
2009-06-04 9:06 ` Andrew Morton
2009-06-04 10:39 ` Minkyu Kang
2009-06-04 10:39 ` Minkyu Kang
2009-06-04 9:16 ` Trilok Soni
2009-06-04 9:16 ` Trilok Soni
2009-06-04 9:35 ` Jean Delvare
2009-06-04 9:35 ` Jean Delvare
2009-06-04 10:55 ` Minkyu Kang
2009-06-04 10:55 ` Minkyu Kang
2009-06-04 10:55 ` Minkyu Kang
2009-06-04 9:35 ` Jean Delvare
2009-06-04 10:47 ` Minkyu Kang
2009-06-04 10:47 ` Minkyu Kang
2009-06-04 10:47 ` Minkyu Kang
2009-06-04 9:16 ` Trilok Soni
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.