[PATCH 1/7] bq27x00: Do not cache current_now value for bq27000 batery

[PATCH 1/7] bq27x00: Do not cache current_now value for bq27000 batery

[Pali Rohár]

* This prevent reporting old current_now value for bq27000
* Also ask for current flags, to make sure that current_now will be reported with correct signature
---
 drivers/power/bq27x00_battery.c |   19 +++++--------------
 1 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index bb16f5b..d238144 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -80,8 +80,6 @@ struct bq27x00_reg_cache {
 	int cycle_count;
 	int capacity;
 	int flags;
-
-	int current_now;
 };
 
 struct bq27x00_device_info {
@@ -270,17 +268,12 @@ static void bq27x00_update(struct bq27x00_device_info *di)
 		cache.charge_full = bq27x00_battery_read_lmd(di);
 		cache.cycle_count = bq27x00_battery_read_cyct(di);
 
-		if (!is_bq27500)
-			cache.current_now = bq27x00_read(di, BQ27x00_REG_AI, false);
-
 		/* We only have to read charge design full once */
 		if (di->charge_design_full <= 0)
 			di->charge_design_full = bq27x00_battery_read_ilmd(di);
 	}
 
-	/* Ignore current_now which is a snapshot of the current battery state
-	 * and is likely to be different even between two consecutive reads */
-	if (memcmp(&di->cache, &cache, sizeof(cache) - sizeof(int)) != 0) {
+	if (memcmp(&di->cache, &cache, sizeof(cache)) != 0) {
 		di->cache = cache;
 		power_supply_changed(&di->bat);
 	}
@@ -330,12 +323,9 @@ static int bq27x00_battery_current(struct bq27x00_device_info *di,
 	union power_supply_propval *val)
 {
 	int curr;
+	int flags;
 
-	if (di->chip == BQ27500)
-	    curr = bq27x00_read(di, BQ27x00_REG_AI, false);
-	else
-	    curr = di->cache.current_now;
-
+	curr = bq27x00_read(di, BQ27x00_REG_AI, false);
 	if (curr < 0)
 		return curr;
 
@@ -343,7 +333,8 @@ static int bq27x00_battery_current(struct bq27x00_device_info *di,
 		/* bq27500 returns signed value */
 		val->intval = (int)((s16)curr) * 1000;
 	} else {
-		if (di->cache.flags & BQ27000_FLAG_CHGS) {
+		flags = bq27x00_read(di, BQ27x00_REG_FLAGS, false);
+		if (flags & BQ27000_FLAG_CHGS) {
 			dev_dbg(di->dev, "negative current!\n");
 			curr = -curr;
 		}
-- 
1.7.4.1

[PATCH 2/7] bq27x00: Add support for property POWER_SUPPLY_PROP_CAPACITY_LEVEL

[Pali Rohár]

---
 drivers/power/bq27x00_battery.c |   40 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index d238144..a531122 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -54,12 +54,16 @@
 
 #define BQ27000_REG_RSOC		0x0B /* Relative State-of-Charge */
 #define BQ27000_REG_ILMD		0x76 /* Initial last measured discharge */
-#define BQ27000_FLAG_CHGS		BIT(7)
+#define BQ27000_FLAG_EDVF		BIT(0) /* Final End-of-Discharge-Voltage flag */
+#define BQ27000_FLAG_EDV1		BIT(1) /* First End-of-Discharge-Voltage flag */
 #define BQ27000_FLAG_FC			BIT(5)
+#define BQ27000_FLAG_CHGS		BIT(7) /* Charge state flag */
 
 #define BQ27500_REG_SOC			0x2C
 #define BQ27500_REG_DCAP		0x3C /* Design capacity */
 #define BQ27500_FLAG_DSC		BIT(0)
+#define BQ27500_FLAG_SOCF		BIT(1) /* State-of-Charge threshold final */
+#define BQ27500_FLAG_SOC1		BIT(2) /* State-of-Charge threshold 1 */
 #define BQ27500_FLAG_FC			BIT(9)
 
 #define BQ27000_RS			20 /* Resistor sense */
@@ -106,6 +110,7 @@ static enum power_supply_property bq27x00_battery_props[] = {
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
 	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_CAPACITY_LEVEL,
 	POWER_SUPPLY_PROP_TEMP,
 	POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW,
 	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
@@ -373,6 +378,36 @@ static int bq27x00_battery_status(struct bq27x00_device_info *di,
 	return 0;
 }
 
+static int bq27x00_battery_capacity_level(struct bq27x00_device_info *di,
+	union power_supply_propval *val)
+{
+	int level;
+
+	if (di->chip == BQ27500) {
+		if (di->cache.flags & BQ27500_FLAG_FC)
+			level = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
+		else if (di->cache.flags & BQ27500_FLAG_SOC1)
+			level = POWER_SUPPLY_CAPACITY_LEVEL_LOW;
+		else if (di->cache.flags & BQ27500_FLAG_SOCF)
+			level = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
+		else
+			level = POWER_SUPPLY_CAPACITY_LEVEL_NORMAL;
+	} else {
+		if (di->cache.flags & BQ27000_FLAG_FC)
+			level = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
+		else if (di->cache.flags & BQ27000_FLAG_EDV1)
+			level = POWER_SUPPLY_CAPACITY_LEVEL_LOW;
+		else if (di->cache.flags & BQ27000_FLAG_EDVF)
+			level = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
+		else
+			level = POWER_SUPPLY_CAPACITY_LEVEL_NORMAL;
+	}
+
+	val->intval = level;
+
+	return 0;
+}
+
 /*
  * Return the battery Voltage in milivolts
  * Or < 0 if something fails.
@@ -464,6 +499,9 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CAPACITY:
 		ret = bq27x00_simple_value(di->cache.capacity, val);
 		break;
+	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
+		ret = bq27x00_battery_capacity_level(di, val);
+		break;
 	case POWER_SUPPLY_PROP_TEMP:
 		ret = bq27x00_battery_temperature(di, val);
 		break;
-- 
1.7.4.1

[PATCH 3/7] bq27x00: Report -ENODATA if bq27000 battery was not calibrated

[Pali Rohár]

* CI (Capacity Inaccurate) flag is set after full reset on bq27000 battery
* when is set, all capacity properties should be reported incorrectly, because there was no learning cycle and battery was not calibrated
* instead reporting incorrect values, report -ENODATA
---
 drivers/power/bq27x00_battery.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index a531122..6c8dfdb 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -56,6 +56,7 @@
 #define BQ27000_REG_ILMD		0x76 /* Initial last measured discharge */
 #define BQ27000_FLAG_EDVF		BIT(0) /* Final End-of-Discharge-Voltage flag */
 #define BQ27000_FLAG_EDV1		BIT(1) /* First End-of-Discharge-Voltage flag */
+#define BQ27000_FLAG_CI			BIT(4) /* Capacity Inaccurate flag */
 #define BQ27000_FLAG_FC			BIT(5)
 #define BQ27000_FLAG_CHGS		BIT(7) /* Charge state flag */
 
@@ -265,12 +266,20 @@ static void bq27x00_update(struct bq27x00_device_info *di)
 
 	cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, is_bq27500);
 	if (cache.flags >= 0) {
-		cache.capacity = bq27x00_battery_read_rsoc(di);
+		if (!is_bq27500 && (cache.flags & BQ27000_FLAG_CI)) {
+			cache.capacity = -ENODATA;
+			cache.time_to_empty = -ENODATA;
+			cache.time_to_empty_avg = -ENODATA;
+			cache.time_to_full = -ENODATA;
+			cache.charge_full = -ENODATA;
+		} else {
+			cache.capacity = bq27x00_battery_read_rsoc(di);
+			cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
+			cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
+			cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
+			cache.charge_full = bq27x00_battery_read_lmd(di);
+		}
 		cache.temperature = bq27x00_read(di, BQ27x00_REG_TEMP, false);
-		cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
-		cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
-		cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
-		cache.charge_full = bq27x00_battery_read_lmd(di);
 		cache.cycle_count = bq27x00_battery_read_cyct(di);
 
 		/* We only have to read charge design full once */
-- 
1.7.4.1

[PATCH 4/7] bq27x00: Cache energy property

[Pali Rohár]

---
 drivers/power/bq27x00_battery.c |   53 +++++++++++++++++++--------------------
 1 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 6c8dfdb..e9aeb53 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -84,6 +84,7 @@ struct bq27x00_reg_cache {
 	int charge_full;
 	int cycle_count;
 	int capacity;
+	int energy;
 	int flags;
 };
 
@@ -225,6 +226,28 @@ static int bq27x00_battery_read_ilmd(struct bq27x00_device_info *di)
 }
 
 /*
+ * Return the battery Available energy in µWh
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_read_energy(struct bq27x00_device_info *di)
+{
+	int ae;
+
+	ae = bq27x00_read(di, BQ27x00_REG_AE, false);
+	if (ae < 0) {
+		dev_err(di->dev, "error reading available energy\n");
+		return ae;
+	}
+
+	if (di->chip == BQ27500)
+		ae *= 1000;
+	else
+		ae = ae * 29200 / BQ27000_RS;
+
+	return ae;
+}
+
+/*
  * Return the battery Cycle count total
  * Or < 0 if something fails.
  */
@@ -268,12 +291,14 @@ static void bq27x00_update(struct bq27x00_device_info *di)
 	if (cache.flags >= 0) {
 		if (!is_bq27500 && (cache.flags & BQ27000_FLAG_CI)) {
 			cache.capacity = -ENODATA;
+			cache.energy = -ENODATA;
 			cache.time_to_empty = -ENODATA;
 			cache.time_to_empty_avg = -ENODATA;
 			cache.time_to_full = -ENODATA;
 			cache.charge_full = -ENODATA;
 		} else {
 			cache.capacity = bq27x00_battery_read_rsoc(di);
+			cache.energy = bq27x00_battery_read_energy(di);
 			cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
 			cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
 			cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
@@ -435,32 +460,6 @@ static int bq27x00_battery_voltage(struct bq27x00_device_info *di,
 	return 0;
 }
 
-/*
- * Return the battery Available energy in µWh
- * Or < 0 if something fails.
- */
-static int bq27x00_battery_energy(struct bq27x00_device_info *di,
-	union power_supply_propval *val)
-{
-	int ae;
-
-	ae = bq27x00_read(di, BQ27x00_REG_AE, false);
-	if (ae < 0) {
-		dev_err(di->dev, "error reading available energy\n");
-		return ae;
-	}
-
-	if (di->chip == BQ27500)
-		ae *= 1000;
-	else
-		ae = ae * 29200 / BQ27000_RS;
-
-	val->intval = ae;
-
-	return 0;
-}
-
-
 static int bq27x00_simple_value(int value,
 	union power_supply_propval *val)
 {
@@ -539,7 +538,7 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
 		ret = bq27x00_simple_value(di->cache.cycle_count, val);
 		break;
 	case POWER_SUPPLY_PROP_ENERGY_NOW:
-		ret = bq27x00_battery_energy(di, val);
+		ret = bq27x00_simple_value(di->cache.energy, val);
 		break;
 	default:
 		return -EINVAL;
-- 
1.7.4.1

[PATCH 5/7] bq27x00: Cache temperature value in converted unit

[Pali Rohár]

---
 drivers/power/bq27x00_battery.c |   45 ++++++++++++++++++++------------------
 1 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index e9aeb53..a22124a 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -248,6 +248,28 @@ static int bq27x00_battery_read_energy(struct bq27x00_device_info *di)
 }
 
 /*
+ * Return the battery temperature in tenths of degree Celsius
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_read_temperature(struct bq27x00_device_info *di)
+{
+	int temp;
+
+	temp = bq27x00_read(di, BQ27x00_REG_TEMP, false);
+	if (temp < 0) {
+		dev_err(di->dev, "error reading temperature\n");
+		return temp;
+	}
+
+	if (di->chip == BQ27500)
+		temp -= 2731;
+	else
+		temp = ((temp * 5) - 5463) / 2;
+
+	return temp;
+}
+
+/*
  * Return the battery Cycle count total
  * Or < 0 if something fails.
  */
@@ -304,7 +326,7 @@ static void bq27x00_update(struct bq27x00_device_info *di)
 			cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
 			cache.charge_full = bq27x00_battery_read_lmd(di);
 		}
-		cache.temperature = bq27x00_read(di, BQ27x00_REG_TEMP, false);
+		cache.temperature = bq27x00_battery_read_temperature(di);
 		cache.cycle_count = bq27x00_battery_read_cyct(di);
 
 		/* We only have to read charge design full once */
@@ -334,25 +356,6 @@ static void bq27x00_battery_poll(struct work_struct *work)
 	}
 }
 
-
-/*
- * Return the battery temperature in tenths of degree Celsius
- * Or < 0 if something fails.
- */
-static int bq27x00_battery_temperature(struct bq27x00_device_info *di,
-	union power_supply_propval *val)
-{
-	if (di->cache.temperature < 0)
-		return di->cache.temperature;
-
-	if (di->chip == BQ27500)
-		val->intval = di->cache.temperature - 2731;
-	else
-		val->intval = ((di->cache.temperature * 5) - 5463) / 2;
-
-	return 0;
-}
-
 /*
  * Return the battery average current in µA
  * Note that current can be negative signed as well
@@ -511,7 +514,7 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
 		ret = bq27x00_battery_capacity_level(di, val);
 		break;
 	case POWER_SUPPLY_PROP_TEMP:
-		ret = bq27x00_battery_temperature(di, val);
+		ret = bq27x00_simple_value(di->cache.temperature, val);
 		break;
 	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
 		ret = bq27x00_simple_value(di->cache.time_to_empty, val);
-- 
1.7.4.1

[PATCH 6/7] bq27x00: Fix reporting status value for bq27500 battery

[Pali Rohár]

---
 drivers/power/bq27x00_battery.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index a22124a..c4c403e 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -62,10 +62,11 @@
 
 #define BQ27500_REG_SOC			0x2C
 #define BQ27500_REG_DCAP		0x3C /* Design capacity */
-#define BQ27500_FLAG_DSC		BIT(0)
+#define BQ27500_FLAG_DSG		BIT(0) /* Discharging */
 #define BQ27500_FLAG_SOCF		BIT(1) /* State-of-Charge threshold final */
 #define BQ27500_FLAG_SOC1		BIT(2) /* State-of-Charge threshold 1 */
-#define BQ27500_FLAG_FC			BIT(9)
+#define BQ27500_FLAG_CHG		BIT(8) /* Charging */
+#define BQ27500_FLAG_FC			BIT(9) /* Fully charged */
 
 #define BQ27000_RS			20 /* Resistor sense */
 
@@ -395,10 +396,14 @@ static int bq27x00_battery_status(struct bq27x00_device_info *di,
 	if (di->chip == BQ27500) {
 		if (di->cache.flags & BQ27500_FLAG_FC)
 			status = POWER_SUPPLY_STATUS_FULL;
-		else if (di->cache.flags & BQ27500_FLAG_DSC)
+		else if (di->cache.flags & BQ27500_FLAG_DSG)
 			status = POWER_SUPPLY_STATUS_DISCHARGING;
-		else
+		else if (di->cache.flags & BQ27500_FLAG_CHG)
 			status = POWER_SUPPLY_STATUS_CHARGING;
+		else if (power_supply_am_i_supplied(&di->bat))
+			status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		else
+			status = POWER_SUPPLY_STATUS_UNKNOWN;
 	} else {
 		if (di->cache.flags & BQ27000_FLAG_FC)
 			status = POWER_SUPPLY_STATUS_FULL;
-- 
1.7.4.1

[PATCH 7/7] bq27x00: Fix reporting error messages

[Pali Rohár]

---
 drivers/power/bq27x00_battery.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index c4c403e..46db966 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -170,7 +170,7 @@ static int bq27x00_battery_read_charge(struct bq27x00_device_info *di, u8 reg)
 
 	charge = bq27x00_read(di, reg, false);
 	if (charge < 0) {
-		dev_err(di->dev, "error reading nominal available capacity\n");
+		dev_err(di->dev, "error reading charge register %02x: %d\n", reg, charge);
 		return charge;
 	}
 
@@ -295,7 +295,7 @@ static int bq27x00_battery_read_time(struct bq27x00_device_info *di, u8 reg)
 
 	tval = bq27x00_read(di, reg, false);
 	if (tval < 0) {
-		dev_err(di->dev, "error reading register %02x: %d\n", reg, tval);
+		dev_err(di->dev, "error reading time register %02x: %d\n", reg, tval);
 		return tval;
 	}
 
@@ -313,6 +313,7 @@ static void bq27x00_update(struct bq27x00_device_info *di)
 	cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, is_bq27500);
 	if (cache.flags >= 0) {
 		if (!is_bq27500 && (cache.flags & BQ27000_FLAG_CI)) {
+			dev_err(di->dev, "battery is not calibrated! ignoring capacity values\n");
 			cache.capacity = -ENODATA;
 			cache.energy = -ENODATA;
 			cache.time_to_empty = -ENODATA;
@@ -369,8 +370,10 @@ static int bq27x00_battery_current(struct bq27x00_device_info *di,
 	int flags;
 
 	curr = bq27x00_read(di, BQ27x00_REG_AI, false);
-	if (curr < 0)
+	if (curr < 0) {
+		dev_err(di->dev, "error reading current");
 		return curr;
+	}
 
 	if (di->chip == BQ27500) {
 		/* bq27500 returns signed value */
@@ -460,8 +463,10 @@ static int bq27x00_battery_voltage(struct bq27x00_device_info *di,
 	int volt;
 
 	volt = bq27x00_read(di, BQ27x00_REG_VOLT, false);
-	if (volt < 0)
+	if (volt < 0) {
+		dev_err(di->dev, "error reading voltage\n");
 		return volt;
+	}
 
 	val->intval = volt * 1000;
 
-- 
1.7.4.1

[PATCH 1/9] bq27x00: Do not cache current_now value for bq27000 batery

[Pali Rohár]

* This prevent reporting old current_now value for bq27000
* Also ask for current flags, to make sure that current_now will be reported with correct signature

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq27x00_battery.c |   19 +++++--------------
 1 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index bb16f5b..d238144 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -80,8 +80,6 @@ struct bq27x00_reg_cache {
 	int cycle_count;
 	int capacity;
 	int flags;
-
-	int current_now;
 };
 
 struct bq27x00_device_info {
@@ -270,17 +268,12 @@ static void bq27x00_update(struct bq27x00_device_info *di)
 		cache.charge_full = bq27x00_battery_read_lmd(di);
 		cache.cycle_count = bq27x00_battery_read_cyct(di);
 
-		if (!is_bq27500)
-			cache.current_now = bq27x00_read(di, BQ27x00_REG_AI, false);
-
 		/* We only have to read charge design full once */
 		if (di->charge_design_full <= 0)
 			di->charge_design_full = bq27x00_battery_read_ilmd(di);
 	}
 
-	/* Ignore current_now which is a snapshot of the current battery state
-	 * and is likely to be different even between two consecutive reads */
-	if (memcmp(&di->cache, &cache, sizeof(cache) - sizeof(int)) != 0) {
+	if (memcmp(&di->cache, &cache, sizeof(cache)) != 0) {
 		di->cache = cache;
 		power_supply_changed(&di->bat);
 	}
@@ -330,12 +323,9 @@ static int bq27x00_battery_current(struct bq27x00_device_info *di,
 	union power_supply_propval *val)
 {
 	int curr;
+	int flags;
 
-	if (di->chip == BQ27500)
-	    curr = bq27x00_read(di, BQ27x00_REG_AI, false);
-	else
-	    curr = di->cache.current_now;
-
+	curr = bq27x00_read(di, BQ27x00_REG_AI, false);
 	if (curr < 0)
 		return curr;
 
@@ -343,7 +333,8 @@ static int bq27x00_battery_current(struct bq27x00_device_info *di,
 		/* bq27500 returns signed value */
 		val->intval = (int)((s16)curr) * 1000;
 	} else {
-		if (di->cache.flags & BQ27000_FLAG_CHGS) {
+		flags = bq27x00_read(di, BQ27x00_REG_FLAGS, false);
+		if (flags & BQ27000_FLAG_CHGS) {
 			dev_dbg(di->dev, "negative current!\n");
 			curr = -curr;
 		}
-- 
1.7.5.4

[PATCH 2/9] bq27x00: Add support for property POWER_SUPPLY_PROP_CAPACITY_LEVEL

[Pali Rohár]

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq27x00_battery.c |   40 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index d238144..a531122 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -54,12 +54,16 @@
 
 #define BQ27000_REG_RSOC		0x0B /* Relative State-of-Charge */
 #define BQ27000_REG_ILMD		0x76 /* Initial last measured discharge */
-#define BQ27000_FLAG_CHGS		BIT(7)
+#define BQ27000_FLAG_EDVF		BIT(0) /* Final End-of-Discharge-Voltage flag */
+#define BQ27000_FLAG_EDV1		BIT(1) /* First End-of-Discharge-Voltage flag */
 #define BQ27000_FLAG_FC			BIT(5)
+#define BQ27000_FLAG_CHGS		BIT(7) /* Charge state flag */
 
 #define BQ27500_REG_SOC			0x2C
 #define BQ27500_REG_DCAP		0x3C /* Design capacity */
 #define BQ27500_FLAG_DSC		BIT(0)
+#define BQ27500_FLAG_SOCF		BIT(1) /* State-of-Charge threshold final */
+#define BQ27500_FLAG_SOC1		BIT(2) /* State-of-Charge threshold 1 */
 #define BQ27500_FLAG_FC			BIT(9)
 
 #define BQ27000_RS			20 /* Resistor sense */
@@ -106,6 +110,7 @@ static enum power_supply_property bq27x00_battery_props[] = {
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
 	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_CAPACITY_LEVEL,
 	POWER_SUPPLY_PROP_TEMP,
 	POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW,
 	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
@@ -373,6 +378,36 @@ static int bq27x00_battery_status(struct bq27x00_device_info *di,
 	return 0;
 }
 
+static int bq27x00_battery_capacity_level(struct bq27x00_device_info *di,
+	union power_supply_propval *val)
+{
+	int level;
+
+	if (di->chip == BQ27500) {
+		if (di->cache.flags & BQ27500_FLAG_FC)
+			level = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
+		else if (di->cache.flags & BQ27500_FLAG_SOC1)
+			level = POWER_SUPPLY_CAPACITY_LEVEL_LOW;
+		else if (di->cache.flags & BQ27500_FLAG_SOCF)
+			level = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
+		else
+			level = POWER_SUPPLY_CAPACITY_LEVEL_NORMAL;
+	} else {
+		if (di->cache.flags & BQ27000_FLAG_FC)
+			level = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
+		else if (di->cache.flags & BQ27000_FLAG_EDV1)
+			level = POWER_SUPPLY_CAPACITY_LEVEL_LOW;
+		else if (di->cache.flags & BQ27000_FLAG_EDVF)
+			level = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
+		else
+			level = POWER_SUPPLY_CAPACITY_LEVEL_NORMAL;
+	}
+
+	val->intval = level;
+
+	return 0;
+}
+
 /*
  * Return the battery Voltage in milivolts
  * Or < 0 if something fails.
@@ -464,6 +499,9 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CAPACITY:
 		ret = bq27x00_simple_value(di->cache.capacity, val);
 		break;
+	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
+		ret = bq27x00_battery_capacity_level(di, val);
+		break;
 	case POWER_SUPPLY_PROP_TEMP:
 		ret = bq27x00_battery_temperature(di, val);
 		break;
-- 
1.7.5.4

[PATCH 3/9] bq27x00: Report -ENODATA if bq27000 battery was not calibrated

[Pali Rohár]

* CI (Capacity Inaccurate) flag is set after full reset on bq27000 battery
* when is set, all capacity properties should be reported incorrectly, because there was no learning cycle and battery was not calibrated
* instead reporting incorrect values, report -ENODATA

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq27x00_battery.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index a531122..6c8dfdb 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -56,6 +56,7 @@
 #define BQ27000_REG_ILMD		0x76 /* Initial last measured discharge */
 #define BQ27000_FLAG_EDVF		BIT(0) /* Final End-of-Discharge-Voltage flag */
 #define BQ27000_FLAG_EDV1		BIT(1) /* First End-of-Discharge-Voltage flag */
+#define BQ27000_FLAG_CI			BIT(4) /* Capacity Inaccurate flag */
 #define BQ27000_FLAG_FC			BIT(5)
 #define BQ27000_FLAG_CHGS		BIT(7) /* Charge state flag */
 
@@ -265,12 +266,20 @@ static void bq27x00_update(struct bq27x00_device_info *di)
 
 	cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, is_bq27500);
 	if (cache.flags >= 0) {
-		cache.capacity = bq27x00_battery_read_rsoc(di);
+		if (!is_bq27500 && (cache.flags & BQ27000_FLAG_CI)) {
+			cache.capacity = -ENODATA;
+			cache.time_to_empty = -ENODATA;
+			cache.time_to_empty_avg = -ENODATA;
+			cache.time_to_full = -ENODATA;
+			cache.charge_full = -ENODATA;
+		} else {
+			cache.capacity = bq27x00_battery_read_rsoc(di);
+			cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
+			cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
+			cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
+			cache.charge_full = bq27x00_battery_read_lmd(di);
+		}
 		cache.temperature = bq27x00_read(di, BQ27x00_REG_TEMP, false);
-		cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
-		cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
-		cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
-		cache.charge_full = bq27x00_battery_read_lmd(di);
 		cache.cycle_count = bq27x00_battery_read_cyct(di);
 
 		/* We only have to read charge design full once */
-- 
1.7.5.4

[PATCH 4/9] bq27x00: Cache energy property

[Pali Rohár]

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq27x00_battery.c |   53 +++++++++++++++++++--------------------
 1 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 6c8dfdb..e9aeb53 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -84,6 +84,7 @@ struct bq27x00_reg_cache {
 	int charge_full;
 	int cycle_count;
 	int capacity;
+	int energy;
 	int flags;
 };
 
@@ -225,6 +226,28 @@ static int bq27x00_battery_read_ilmd(struct bq27x00_device_info *di)
 }
 
 /*
+ * Return the battery Available energy in µWh
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_read_energy(struct bq27x00_device_info *di)
+{
+	int ae;
+
+	ae = bq27x00_read(di, BQ27x00_REG_AE, false);
+	if (ae < 0) {
+		dev_err(di->dev, "error reading available energy\n");
+		return ae;
+	}
+
+	if (di->chip == BQ27500)
+		ae *= 1000;
+	else
+		ae = ae * 29200 / BQ27000_RS;
+
+	return ae;
+}
+
+/*
  * Return the battery Cycle count total
  * Or < 0 if something fails.
  */
@@ -268,12 +291,14 @@ static void bq27x00_update(struct bq27x00_device_info *di)
 	if (cache.flags >= 0) {
 		if (!is_bq27500 && (cache.flags & BQ27000_FLAG_CI)) {
 			cache.capacity = -ENODATA;
+			cache.energy = -ENODATA;
 			cache.time_to_empty = -ENODATA;
 			cache.time_to_empty_avg = -ENODATA;
 			cache.time_to_full = -ENODATA;
 			cache.charge_full = -ENODATA;
 		} else {
 			cache.capacity = bq27x00_battery_read_rsoc(di);
+			cache.energy = bq27x00_battery_read_energy(di);
 			cache.time_to_empty = bq27x00_battery_read_time(di, BQ27x00_REG_TTE);
 			cache.time_to_empty_avg = bq27x00_battery_read_time(di, BQ27x00_REG_TTECP);
 			cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
@@ -435,32 +460,6 @@ static int bq27x00_battery_voltage(struct bq27x00_device_info *di,
 	return 0;
 }
 
-/*
- * Return the battery Available energy in µWh
- * Or < 0 if something fails.
- */
-static int bq27x00_battery_energy(struct bq27x00_device_info *di,
-	union power_supply_propval *val)
-{
-	int ae;
-
-	ae = bq27x00_read(di, BQ27x00_REG_AE, false);
-	if (ae < 0) {
-		dev_err(di->dev, "error reading available energy\n");
-		return ae;
-	}
-
-	if (di->chip == BQ27500)
-		ae *= 1000;
-	else
-		ae = ae * 29200 / BQ27000_RS;
-
-	val->intval = ae;
-
-	return 0;
-}
-
-
 static int bq27x00_simple_value(int value,
 	union power_supply_propval *val)
 {
@@ -539,7 +538,7 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
 		ret = bq27x00_simple_value(di->cache.cycle_count, val);
 		break;
 	case POWER_SUPPLY_PROP_ENERGY_NOW:
-		ret = bq27x00_battery_energy(di, val);
+		ret = bq27x00_simple_value(di->cache.energy, val);
 		break;
 	default:
 		return -EINVAL;
-- 
1.7.5.4

[PATCH 5/9] bq27x00: Cache temperature value in converted unit

[Pali Rohár]

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq27x00_battery.c |   45 ++++++++++++++++++++------------------
 1 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index e9aeb53..a22124a 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -248,6 +248,28 @@ static int bq27x00_battery_read_energy(struct bq27x00_device_info *di)
 }
 
 /*
+ * Return the battery temperature in tenths of degree Celsius
+ * Or < 0 if something fails.
+ */
+static int bq27x00_battery_read_temperature(struct bq27x00_device_info *di)
+{
+	int temp;
+
+	temp = bq27x00_read(di, BQ27x00_REG_TEMP, false);
+	if (temp < 0) {
+		dev_err(di->dev, "error reading temperature\n");
+		return temp;
+	}
+
+	if (di->chip == BQ27500)
+		temp -= 2731;
+	else
+		temp = ((temp * 5) - 5463) / 2;
+
+	return temp;
+}
+
+/*
  * Return the battery Cycle count total
  * Or < 0 if something fails.
  */
@@ -304,7 +326,7 @@ static void bq27x00_update(struct bq27x00_device_info *di)
 			cache.time_to_full = bq27x00_battery_read_time(di, BQ27x00_REG_TTF);
 			cache.charge_full = bq27x00_battery_read_lmd(di);
 		}
-		cache.temperature = bq27x00_read(di, BQ27x00_REG_TEMP, false);
+		cache.temperature = bq27x00_battery_read_temperature(di);
 		cache.cycle_count = bq27x00_battery_read_cyct(di);
 
 		/* We only have to read charge design full once */
@@ -334,25 +356,6 @@ static void bq27x00_battery_poll(struct work_struct *work)
 	}
 }
 
-
-/*
- * Return the battery temperature in tenths of degree Celsius
- * Or < 0 if something fails.
- */
-static int bq27x00_battery_temperature(struct bq27x00_device_info *di,
-	union power_supply_propval *val)
-{
-	if (di->cache.temperature < 0)
-		return di->cache.temperature;
-
-	if (di->chip == BQ27500)
-		val->intval = di->cache.temperature - 2731;
-	else
-		val->intval = ((di->cache.temperature * 5) - 5463) / 2;
-
-	return 0;
-}
-
 /*
  * Return the battery average current in µA
  * Note that current can be negative signed as well
@@ -511,7 +514,7 @@ static int bq27x00_battery_get_property(struct power_supply *psy,
 		ret = bq27x00_battery_capacity_level(di, val);
 		break;
 	case POWER_SUPPLY_PROP_TEMP:
-		ret = bq27x00_battery_temperature(di, val);
+		ret = bq27x00_simple_value(di->cache.temperature, val);
 		break;
 	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
 		ret = bq27x00_simple_value(di->cache.time_to_empty, val);
-- 
1.7.5.4

[PATCH 6/9] bq27x00: Fix reporting status value for bq27500 battery

[Pali Rohár]

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq27x00_battery.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index a22124a..c4c403e 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -62,10 +62,11 @@
 
 #define BQ27500_REG_SOC			0x2C
 #define BQ27500_REG_DCAP		0x3C /* Design capacity */
-#define BQ27500_FLAG_DSC		BIT(0)
+#define BQ27500_FLAG_DSG		BIT(0) /* Discharging */
 #define BQ27500_FLAG_SOCF		BIT(1) /* State-of-Charge threshold final */
 #define BQ27500_FLAG_SOC1		BIT(2) /* State-of-Charge threshold 1 */
-#define BQ27500_FLAG_FC			BIT(9)
+#define BQ27500_FLAG_CHG		BIT(8) /* Charging */
+#define BQ27500_FLAG_FC			BIT(9) /* Fully charged */
 
 #define BQ27000_RS			20 /* Resistor sense */
 
@@ -395,10 +396,14 @@ static int bq27x00_battery_status(struct bq27x00_device_info *di,
 	if (di->chip == BQ27500) {
 		if (di->cache.flags & BQ27500_FLAG_FC)
 			status = POWER_SUPPLY_STATUS_FULL;
-		else if (di->cache.flags & BQ27500_FLAG_DSC)
+		else if (di->cache.flags & BQ27500_FLAG_DSG)
 			status = POWER_SUPPLY_STATUS_DISCHARGING;
-		else
+		else if (di->cache.flags & BQ27500_FLAG_CHG)
 			status = POWER_SUPPLY_STATUS_CHARGING;
+		else if (power_supply_am_i_supplied(&di->bat))
+			status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		else
+			status = POWER_SUPPLY_STATUS_UNKNOWN;
 	} else {
 		if (di->cache.flags & BQ27000_FLAG_FC)
 			status = POWER_SUPPLY_STATUS_FULL;
-- 
1.7.5.4

[PATCH 7/9] bq27x00: Fix reporting error messages

[Pali Rohár]

* Do not be noise if battery is not calibrated (use dev_dbg)

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq27x00_battery.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index c4c403e..63cfb5a 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -155,7 +155,7 @@ static int bq27x00_battery_read_rsoc(struct bq27x00_device_info *di)
 		rsoc = bq27x00_read(di, BQ27000_REG_RSOC, true);
 
 	if (rsoc < 0)
-		dev_err(di->dev, "error reading relative State-of-Charge\n");
+		dev_dbg(di->dev, "error reading relative State-of-Charge\n");
 
 	return rsoc;
 }
@@ -170,7 +170,7 @@ static int bq27x00_battery_read_charge(struct bq27x00_device_info *di, u8 reg)
 
 	charge = bq27x00_read(di, reg, false);
 	if (charge < 0) {
-		dev_err(di->dev, "error reading nominal available capacity\n");
+		dev_dbg(di->dev, "error reading charge register %02x: %d\n", reg, charge);
 		return charge;
 	}
 
@@ -214,7 +214,7 @@ static int bq27x00_battery_read_ilmd(struct bq27x00_device_info *di)
 		ilmd = bq27x00_read(di, BQ27000_REG_ILMD, true);
 
 	if (ilmd < 0) {
-		dev_err(di->dev, "error reading initial last measured discharge\n");
+		dev_dbg(di->dev, "error reading initial last measured discharge\n");
 		return ilmd;
 	}
 
@@ -236,7 +236,7 @@ static int bq27x00_battery_read_energy(struct bq27x00_device_info *di)
 
 	ae = bq27x00_read(di, BQ27x00_REG_AE, false);
 	if (ae < 0) {
-		dev_err(di->dev, "error reading available energy\n");
+		dev_dbg(di->dev, "error reading available energy\n");
 		return ae;
 	}
 
@@ -295,7 +295,7 @@ static int bq27x00_battery_read_time(struct bq27x00_device_info *di, u8 reg)
 
 	tval = bq27x00_read(di, reg, false);
 	if (tval < 0) {
-		dev_err(di->dev, "error reading register %02x: %d\n", reg, tval);
+		dev_dbg(di->dev, "error reading time register %02x: %d\n", reg, tval);
 		return tval;
 	}
 
@@ -313,6 +313,7 @@ static void bq27x00_update(struct bq27x00_device_info *di)
 	cache.flags = bq27x00_read(di, BQ27x00_REG_FLAGS, is_bq27500);
 	if (cache.flags >= 0) {
 		if (!is_bq27500 && (cache.flags & BQ27000_FLAG_CI)) {
+			dev_info(di->dev, "battery is not calibrated! ignoring capacity values\n");
 			cache.capacity = -ENODATA;
 			cache.energy = -ENODATA;
 			cache.time_to_empty = -ENODATA;
@@ -369,8 +370,10 @@ static int bq27x00_battery_current(struct bq27x00_device_info *di,
 	int flags;
 
 	curr = bq27x00_read(di, BQ27x00_REG_AI, false);
-	if (curr < 0)
+	if (curr < 0) {
+		dev_err(di->dev, "error reading current\n");
 		return curr;
+	}
 
 	if (di->chip == BQ27500) {
 		/* bq27500 returns signed value */
@@ -460,8 +463,10 @@ static int bq27x00_battery_voltage(struct bq27x00_device_info *di,
 	int volt;
 
 	volt = bq27x00_read(di, BQ27x00_REG_VOLT, false);
-	if (volt < 0)
+	if (volt < 0) {
+		dev_err(di->dev, "error reading voltage\n");
 		return volt;
+	}
 
 	val->intval = volt * 1000;
 
-- 
1.7.5.4

[PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Pali Rohár]

* When bq27x00_battery module is loaded it take control of bq27200 i2c battery chip and then it is not possible to use i2cget program from user space (returns -EBUSY)
* This patch adds new miscdevice for each battery which has ioctl BQ27X00_READ_REG for reading battery registers from user space

* ioctl cmd:
  #define BQ27X00_READ_REG _IO(MISC_MAJOR, 0)

* ioctl arg:
  struct bq27x00_reg_parms {
    int reg; /* battery register (in) */
    int single; /* 1 - 8bit register, 0 - 16bit register (in) */
    int ret; /* register status, negative indicate error (out) */
  };

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq27x00_battery.c       |  136 +++++++++++++++++++++++++++++++--
 include/linux/power/bq27x00_battery.h |   15 ++++
 2 files changed, 145 insertions(+), 6 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 63cfb5a..15ecd42 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -29,12 +29,15 @@
 #include <linux/jiffies.h>
 #include <linux/workqueue.h>
 #include <linux/delay.h>
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/idr.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <asm/unaligned.h>
+#include <asm/uaccess.h>
 
 #include <linux/power/bq27x00_battery.h>
 
@@ -89,8 +92,15 @@ struct bq27x00_reg_cache {
 	int flags;
 };
 
+struct bq27x00_reg_device {
+	struct miscdevice miscdev;
+	struct bq27x00_device_info *di;
+	struct list_head list;
+};
+
 struct bq27x00_device_info {
 	struct device 		*dev;
+	struct bq27x00_reg_device *regdev;
 	int			id;
 	enum bq27x00_chip	chip;
 
@@ -131,6 +141,13 @@ module_param(poll_interval, uint, 0644);
 MODULE_PARM_DESC(poll_interval, "battery poll interval in seconds - " \
 				"0 disables polling");
 
+/* If the system has several batteries we need a different name for each
+ * of them...
+ */
+static DEFINE_IDR(battery_id);
+static DEFINE_MUTEX(battery_mutex);
+static LIST_HEAD(battery_list);
+
 /*
  * Common code for BQ27x00 devices
  */
@@ -568,6 +585,114 @@ static void bq27x00_external_power_changed(struct power_supply *psy)
 	schedule_delayed_work(&di->work, 0);
 }
 
+/* Code for register device access */
+
+static struct bq27x00_reg_device * bq27x00_battery_reg_find_device(int minor)
+{
+	struct bq27x00_reg_device *regdev;
+
+	list_for_each_entry(regdev, &battery_list, list)
+		if (regdev->miscdev.minor == minor)
+			return regdev;
+
+	return NULL;
+}
+
+static long bq27x00_battery_reg_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	int ret;
+	int minor = iminor(filp->f_dentry->d_inode);
+	struct bq27x00_reg_parms param;
+	struct bq27x00_reg_device *regdev = bq27x00_battery_reg_find_device(minor);
+
+	if (!regdev)
+		return -ENXIO;
+
+	if (cmd != BQ27X00_READ_REG)
+		return -EINVAL;
+
+	ret = copy_from_user(&param, (void __user *)arg, sizeof(param));
+	if (ret != 0)
+		return -EACCES;
+
+	param.ret = bq27x00_read(regdev->di, param.reg, param.single);
+
+	ret = copy_to_user((void __user *)arg, &param, sizeof(param));
+	if (ret != 0)
+		return -EACCES;
+
+	return 0;
+}
+
+static int bq27x00_battery_reg_open(struct inode *inode, struct file *file)
+{
+	if (!try_module_get(THIS_MODULE))
+		return -EPERM;
+
+	return 0;
+}
+
+static int bq27x00_battery_reg_release(struct inode *inode, struct file *file)
+{
+	module_put(THIS_MODULE);
+	return 0;
+}
+
+static struct file_operations bq27x00_reg_fileops = {
+	.owner = THIS_MODULE,
+	.unlocked_ioctl = bq27x00_battery_reg_ioctl,
+	.open = bq27x00_battery_reg_open,
+	.release = bq27x00_battery_reg_release,
+};
+
+static int bq27x00_battery_reg_init(struct bq27x00_device_info *di)
+{
+	struct bq27x00_reg_device *regdev;
+	int ret;
+
+	di->regdev = NULL;
+
+	regdev = kzalloc(sizeof *regdev, GFP_KERNEL);
+	if (!regdev)
+		return -ENOMEM;
+
+	regdev->miscdev.minor = MISC_DYNAMIC_MINOR;
+	regdev->miscdev.name = di->bat.name;
+	regdev->miscdev.fops = &bq27x00_reg_fileops;
+
+	ret = misc_register(&regdev->miscdev);
+	if (ret != 0) {
+		kfree(regdev);
+		return ret;
+	}
+
+	regdev->di = di;
+	di->regdev = regdev;
+
+	INIT_LIST_HEAD(&regdev->list);
+
+	mutex_lock(&battery_mutex);
+	list_add(&regdev->list, &battery_list);
+	mutex_unlock(&battery_mutex);
+
+	return 0;
+}
+
+static void bq27x00_battery_reg_exit(struct bq27x00_device_info *di)
+{
+	if (!di->regdev)
+		return;
+
+	misc_deregister(&di->regdev->miscdev);
+
+	mutex_lock(&battery_mutex);
+	list_del(&di->regdev->list);
+	mutex_unlock(&battery_mutex);
+
+	kfree(di->regdev);
+	di->regdev = NULL;
+}
+
 static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
 {
 	int ret;
@@ -590,6 +715,7 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
 	dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
 
 	bq27x00_update(di);
+	bq27x00_battery_reg_init(di);
 
 	return 0;
 }
@@ -598,6 +724,8 @@ static void bq27x00_powersupply_unregister(struct bq27x00_device_info *di)
 {
 	cancel_delayed_work_sync(&di->work);
 
+	bq27x00_battery_reg_exit(di);
+
 	power_supply_unregister(&di->bat);
 
 	mutex_destroy(&di->lock);
@@ -607,12 +735,6 @@ static void bq27x00_powersupply_unregister(struct bq27x00_device_info *di)
 /* i2c specific code */
 #ifdef CONFIG_BATTERY_BQ27X00_I2C
 
-/* If the system has several batteries we need a different name for each
- * of them...
- */
-static DEFINE_IDR(battery_id);
-static DEFINE_MUTEX(battery_mutex);
-
 static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg, bool single)
 {
 	struct i2c_client *client = to_i2c_client(di->dev);
@@ -635,7 +757,9 @@ static int bq27x00_read_i2c(struct bq27x00_device_info *di, u8 reg, bool single)
 	else
 		msg[1].len = 2;
 
+	mutex_lock(&battery_mutex);
 	ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+	mutex_unlock(&battery_mutex);
 	if (ret < 0)
 		return ret;
 
diff --git a/include/linux/power/bq27x00_battery.h b/include/linux/power/bq27x00_battery.h
index a857f71..02c1c6e 100644
--- a/include/linux/power/bq27x00_battery.h
+++ b/include/linux/power/bq27x00_battery.h
@@ -16,4 +16,19 @@ struct bq27000_platform_data {
 	int (*read)(struct device *dev, unsigned int);
 };
 
+
+#define BQ27X00_READ_REG _IO(MISC_MAJOR, 0)
+
+/**
+ * struct bq27x00_reg_params - User space data for ioctl BQ27X00_READ_REG
+ * @reg: Battery register (in)
+ * @single: 1 - 8bit register, 0 - 16bit register (in)
+ * @ret: register status, negative indicate error (out)
+ */
+struct bq27x00_reg_parms {
+	int reg;
+	int single;
+	int ret;
+};
+
 #endif
-- 
1.7.5.4

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Lars-Peter Clausen]

On 11/01/2011 01:43 AM, Pali Rohár wrote:
> * When bq27x00_battery module is loaded it take control of bq27200 i2c battery chip and then it is not possible to use i2cget program from user space (returns -EBUSY)
> * This patch adds new miscdevice for each battery which has ioctl BQ27X00_READ_REG for reading battery registers from user space
> 
> * ioctl cmd:
>   #define BQ27X00_READ_REG _IO(MISC_MAJOR, 0)
> 
> * ioctl arg:
>   struct bq27x00_reg_parms {
>     int reg; /* battery register (in) */
>     int single; /* 1 - 8bit register, 0 - 16bit register (in) */
>     int ret; /* register status, negative indicate error (out) */
>   };
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

I'm not sure if this is such a good idea. Why do you need to read the registers
from userspace? Shouldn't all relevant information be available in a
standardized format through the power_supply userspace API? If you want to use
it for debugging it might be a better idea to provide a debugfs file.

- Lars

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Pali Rohár]

2011/11/1 Lars-Peter Clausen <lars@metafoo.de>:
> On 11/01/2011 01:43 AM, Pali Rohár wrote:
>> * When bq27x00_battery module is loaded it take control of bq27200 i2c battery chip and then it is not possible to use i2cget program from user space (returns -EBUSY)
>> * This patch adds new miscdevice for each battery which has ioctl BQ27X00_READ_REG for reading battery registers from user space
>>
>> * ioctl cmd:
>>   #define BQ27X00_READ_REG _IO(MISC_MAJOR, 0)
>>
>> * ioctl arg:
>>   struct bq27x00_reg_parms {
>>     int reg; /* battery register (in) */
>>     int single; /* 1 - 8bit register, 0 - 16bit register (in) */
>>     int ret; /* register status, negative indicate error (out) */
>>   };
>>
>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
>
> I'm not sure if this is such a good idea. Why do you need to read the registers
> from userspace? Shouldn't all relevant information be available in a
> standardized format through the power_supply userspace API? If you want to use
> it for debugging it might be a better idea to provide a debugfs file.
>
> - Lars
>

bq27200 chip has a lot of registers (for diagnostic) which is usefull
on Nokia N900. power_supply interface is not very abstract to handle
all of them. Also on Nokia N900 is needed proprietary pogram BME for
battery charging which need to access to bq27200 registers. A simple
LD_PRELOAD library which wrap i2c-dev access to this device provided
by this patch is here:
https://code.launchpad.net/~pali/+junk/maemo_libbqioctl

Other shell script which show all info from bq register is here:
http://enivax.net/jk/n900/bq.tar

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Lars-Peter Clausen]

On 11/01/2011 01:23 PM, Pali Rohár wrote:
> 2011/11/1 Lars-Peter Clausen <lars@metafoo.de>:
>> On 11/01/2011 01:43 AM, Pali Rohár wrote:
>>> * When bq27x00_battery module is loaded it take control of bq27200 i2c battery chip and then it is not possible to use i2cget program from user space (returns -EBUSY)
>>> * This patch adds new miscdevice for each battery which has ioctl BQ27X00_READ_REG for reading battery registers from user space
>>>
>>> * ioctl cmd:
>>>   #define BQ27X00_READ_REG _IO(MISC_MAJOR, 0)
>>>
>>> * ioctl arg:
>>>   struct bq27x00_reg_parms {
>>>     int reg; /* battery register (in) */
>>>     int single; /* 1 - 8bit register, 0 - 16bit register (in) */
>>>     int ret; /* register status, negative indicate error (out) */
>>>   };
>>>
>>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
>>
>> I'm not sure if this is such a good idea. Why do you need to read the registers
>> from userspace? Shouldn't all relevant information be available in a
>> standardized format through the power_supply userspace API? If you want to use
>> it for debugging it might be a better idea to provide a debugfs file.
>>
>> - Lars
>>
> 
> bq27200 chip has a lot of registers (for diagnostic) which is usefull
> on Nokia N900. power_supply interface is not very abstract to handle
> all of them. 

Which registers are we talking about here. And if this generally useful
information does it make sense to add a new property to the power_supply interface?

> Also on Nokia N900 is needed proprietary pogram BME for
> battery charging which need to access to bq27200 registers. A simple
> LD_PRELOAD library which wrap i2c-dev access to this device provided
> by this patch is here:
> https://code.launchpad.net/~pali/+junk/maemo_libbqioctl

Yes, but that really is a hack to work around the closed source nature of the
BME program. I don't think we should put such hacks into the kernel.

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Pali Rohár]

2011/11/1 Lars-Peter Clausen <lars@metafoo.de>:
> On 11/01/2011 01:23 PM, Pali Rohár wrote:
>> 2011/11/1 Lars-Peter Clausen <lars@metafoo.de>:
>>> On 11/01/2011 01:43 AM, Pali Rohár wrote:
>>>> * When bq27x00_battery module is loaded it take control of bq27200 i2c battery chip and then it is not possible to use i2cget program from user space (returns -EBUSY)
>>>> * This patch adds new miscdevice for each battery which has ioctl BQ27X00_READ_REG for reading battery registers from user space
>>>>
>>>> * ioctl cmd:
>>>>   #define BQ27X00_READ_REG _IO(MISC_MAJOR, 0)
>>>>
>>>> * ioctl arg:
>>>>   struct bq27x00_reg_parms {
>>>>     int reg; /* battery register (in) */
>>>>     int single; /* 1 - 8bit register, 0 - 16bit register (in) */
>>>>     int ret; /* register status, negative indicate error (out) */
>>>>   };
>>>>
>>>> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
>>>
>>> I'm not sure if this is such a good idea. Why do you need to read the registers
>>> from userspace? Shouldn't all relevant information be available in a
>>> standardized format through the power_supply userspace API? If you want to use
>>> it for debugging it might be a better idea to provide a debugfs file.
>>>
>>> - Lars
>>>
>>
>> bq27200 chip has a lot of registers (for diagnostic) which is usefull
>> on Nokia N900. power_supply interface is not very abstract to handle
>> all of them.
>
> Which registers are we talking about here. And if this generally useful
> information does it make sense to add a new property to the power_supply interface?

I think it is usefull only for bq, see shell script
http://enivax.net/jk/n900/bq.tar

>
>> Also on Nokia N900 is needed proprietary pogram BME for
>> battery charging which need to access to bq27200 registers. A simple
>> LD_PRELOAD library which wrap i2c-dev access to this device provided
>> by this patch is here:
>> https://code.launchpad.net/~pali/+junk/maemo_libbqioctl
>
> Yes, but that really is a hack to work around the closed source nature of the
> BME program. I don't think we should put such hacks into the kernel.
>

Not only BME. All programs (and shell scripts and all which use
i2cget) need this interface, bacuse loading bq module will block
ic2-dev.

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Anton Vorontsov]

Hello Pali,

On Tue, Nov 01, 2011 at 01:53:26PM +0100, Pali Rohár wrote:
[...]
> >> Also on Nokia N900 is needed proprietary pogram BME for
> >> battery charging which need to access to bq27200 registers. A simple
> >> LD_PRELOAD library which wrap i2c-dev access to this device provided
> >> by this patch is here:
> >> https://code.launchpad.net/~pali/+junk/maemo_libbqioctl
> >
> > Yes, but that really is a hack to work around the closed source nature of the
> > BME program. I don't think we should put such hacks into the kernel.
> >
> 
> Not only BME. All programs (and shell scripts and all which use
> i2cget) need this interface, bacuse loading bq module will block
> ic2-dev.

A million thanks for your work! Though, on this particular patch I
agree with Lars-Peter here. The raw abi interface ought to exist only
because proprietary nature of BME, which is not good.

IIRC, there is some shell script that makes it possible to charge
N900 without BME? Is it possible to convert it to some "n900 charger
driver"?

I applied all patches from this series, except this one.

Thanks,

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 2568 bytes --]

On Saturday 26 November 2011 00:10:11 you wrote:
> Hello Pali,

Hello,

> 
> On Tue, Nov 01, 2011 at 01:53:26PM +0100, Pali Rohár wrote:
> [...]
> 
> > >> Also on Nokia N900 is needed proprietary pogram BME for
> > >> battery charging which need to access to bq27200 registers. A
> > >> simple
> > >> LD_PRELOAD library which wrap i2c-dev access to this device
> > >> provided
> > >> by this patch is here:
> > >> https://code.launchpad.net/~pali/+junk/maemo_libbqioctl
> > > 
> > > Yes, but that really is a hack to work around the closed source
> > > nature of the BME program. I don't think we should put such hacks
> > > into the kernel.> 
> > Not only BME. All programs (and shell scripts and all which use
> > i2cget) need this interface, bacuse loading bq module will block
> > ic2-dev.
> 
> A million thanks for your work! Though, on this particular patch I
> agree with Lars-Peter here. The raw abi interface ought to exist only
> because proprietary nature of BME, which is not good.

This interface is not only for BME. Also some popular bq27200.sh script which 
print bq registers in human readable form needs this interface (with 
LD_PRELOAD library).

Link for that shell script http://enivax.net/jk/n900/bq.tar

> 
> IIRC, there is some shell script that makes it possible to charge
> N900 without BME?

bq27200 chip is for retrieving informations about battery. Chip bq24150 is 
responsible for charging battery. And yes, there is shell script which use 
this chip and can charge N900 battery (when BME is stopped).

Link: http://enivax.net/jk/n900/charge21.sh.txt

Problem is that this charge script need access to bq27200 registers too and 
the only way is again rmmod bq27x00_battery driver or using that my interface.

> Is it possible to convert it to some "n900 charger driver"?

And two hours ago I started writing kernel driver for chip bq2415x which will 
support charging battery. So I think yes!

So I think, one day I will delete BME from my Nokia N900.

> 
> I applied all patches from this series, except this one.
> 
> Thanks,

Ok, thanks!

I need some interface from userspace which can access to bq27200 registers. 
Problem is that if driver bq27x00_battery is loaded it is not possible to 
access to registers via i2c-tools. If this patch is not acceptable to 
upstream, what is solution to this problem? I think that it is not good idea 
to rmmod bq27x00_batery driver before executing scripts and then modprobing it 
again...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Anton Vorontsov]

On Fri, Nov 25, 2011 at 09:30:28PM +0100, Pali Rohár wrote:
[...]
> This interface is not only for BME. Also some popular bq27200.sh script which 
> print bq registers in human readable form needs this interface (with 
> LD_PRELOAD library).
> 
> Link for that shell script http://enivax.net/jk/n900/bq.tar

That might be a good excuse to have the raw interface. Although,
as this is for debugging purposes only, the same effect can be
accomplished by unloading bq module and using i2c userspace
interface directly... I guess.

[...]
> > Is it possible to convert it to some "n900 charger driver"?
> 
> And two hours ago I started writing kernel driver for chip bq2415x which will 
> support charging battery. So I think yes!
> 
> So I think, one day I will delete BME from my Nokia N900.

That's a wonderful news! I really look forward to seeing it and
as n900 owner I very much appreciate your work.

Thanks!

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

On Saturday 26 November 2011 00:46:26 Anton Vorontsov wrote:
> On Fri, Nov 25, 2011 at 09:30:28PM +0100, Pali Rohár wrote:
> [...]
> 
> > This interface is not only for BME. Also some popular bq27200.sh script
> > which print bq registers in human readable form needs this interface
> > (with LD_PRELOAD library).
> > 
> > Link for that shell script http://enivax.net/jk/n900/bq.tar
> 
> That might be a good excuse to have the raw interface. Although,
> as this is for debugging purposes only, the same effect can be
> accomplished by unloading bq module and using i2c userspace
> interface directly... I guess.
> 

Yes, unloading bq module and then starting script working. But I think that we 
could have some interface how to access directly to i2c when some i2c module 
for chip is loaded.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Anton Vorontsov]

On Fri, Nov 25, 2011 at 09:54:04PM +0100, Pali Rohár wrote:
> On Saturday 26 November 2011 00:46:26 Anton Vorontsov wrote:
> > On Fri, Nov 25, 2011 at 09:30:28PM +0100, Pali Rohár wrote:
> > [...]
> > 
> > > This interface is not only for BME. Also some popular bq27200.sh script
> > > which print bq registers in human readable form needs this interface
> > > (with LD_PRELOAD library).
> > > 
> > > Link for that shell script http://enivax.net/jk/n900/bq.tar
> > 
> > That might be a good excuse to have the raw interface. Although,
> > as this is for debugging purposes only, the same effect can be
> > accomplished by unloading bq module and using i2c userspace
> > interface directly... I guess.
> > 
> 
> Yes, unloading bq module and then starting script working. But I think that we 
> could have some interface how to access directly to i2c when some i2c module 
> for chip is loaded.

This would be not safe as this might (in case of RW registers)
break kernel's driver behaviour (well, in bq case you only allow
reading, so not problem in this particular case).

What would be more practical, is to allow I2C core to provide
userspace interface even for already bound I2C devices.

That could be some kind of CONFIG_I2C_UNSAFE_DEBUG: when
selected I2C core would allow access to all I2C devices. But still,
the niche for such a feature is tiny, so I doubt that it is worth
doing at all.

In any case, I just think that being able to access already bound
I2C devices from userspace might be a good thing for debugging,
but having such an interface per-driver is impractical.

Thanks,

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 1952 bytes --]

On Saturday 26 November 2011 02:04:48 Anton Vorontsov wrote:
> On Fri, Nov 25, 2011 at 09:54:04PM +0100, Pali Rohár wrote:
> > On Saturday 26 November 2011 00:46:26 Anton Vorontsov wrote:
> > > On Fri, Nov 25, 2011 at 09:30:28PM +0100, Pali Rohár wrote:
> > > [...]
> > > 
> > > > This interface is not only for BME. Also some popular bq27200.sh
> > > > script which print bq registers in human readable form needs
> > > > this interface (with LD_PRELOAD library).
> > > > 
> > > > Link for that shell script http://enivax.net/jk/n900/bq.tar
> > > 
> > > That might be a good excuse to have the raw interface. Although,
> > > as this is for debugging purposes only, the same effect can be
> > > accomplished by unloading bq module and using i2c userspace
> > > interface directly... I guess.
> > 
> > Yes, unloading bq module and then starting script working. But I think
> > that we could have some interface how to access directly to i2c when
> > some i2c module for chip is loaded.
> 
> This would be not safe as this might (in case of RW registers)
> break kernel's driver behaviour (well, in bq case you only allow
> reading, so not problem in this particular case).
> 
> What would be more practical, is to allow I2C core to provide
> userspace interface even for already bound I2C devices.
> 
> That could be some kind of CONFIG_I2C_UNSAFE_DEBUG: when
> selected I2C core would allow access to all I2C devices. But still,
> the niche for such a feature is tiny, so I doubt that it is worth
> doing at all.
> 
> In any case, I just think that being able to access already bound
> I2C devices from userspace might be a good thing for debugging,
> but having such an interface per-driver is impractical.
> 
> Thanks,

Yes, you are right that per-driver access is bad. So I will use this ioctl 
interface patch only in maemo specified kernels (eg. maemo kernel-power).

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Mark Brown]

On Sat, Nov 26, 2011 at 02:04:48AM +0400, Anton Vorontsov wrote:

> What would be more practical, is to allow I2C core to provide
> userspace interface even for already bound I2C devices.

> That could be some kind of CONFIG_I2C_UNSAFE_DEBUG: when
> selected I2C core would allow access to all I2C devices. But still,
> the niche for such a feature is tiny, so I doubt that it is worth
> doing at all.

I don't know if it's applicable here but if this is a device that has
registers that fit within regmap then the regmap API provides a debugfs
interface for dumping the register map as standard.

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 1356 bytes --]

On Sunday 27 November 2011 11:54:16 Mark Brown wrote:
> On Sat, Nov 26, 2011 at 02:04:48AM +0400, Anton Vorontsov wrote:
> > What would be more practical, is to allow I2C core to provide
> > userspace interface even for already bound I2C devices.
> > 
> > That could be some kind of CONFIG_I2C_UNSAFE_DEBUG: when
> > selected I2C core would allow access to all I2C devices. But still,
> > the niche for such a feature is tiny, so I doubt that it is worth
> > doing at all.
> 
> I don't know if it's applicable here but if this is a device that has
> registers that fit within regmap then the regmap API provides a debugfs
> interface for dumping the register map as standard.

I do not know regmap api, but I need interface which is easy readable from C 
or shell scripts and which can read some registers (numbers 0-N).

These patches I written for Nokia N900. Now I need interface available in 
kernel 2.6.28, which is only version working with Maemo (default N900 system). 
Of course upstreaming *all* N900 drivers is needed, because I belive that one 
day I will download vanilla kernel and run it on N900.


Anton Vorontsov, is simple debugfs interface (files - or only one file - which 
have names of register numbers) acceptable for upstreaming (instead this 
miscdevice with ioctl)?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 8/9] bq27x00: Add miscdevice for each battery with ioctl for reading registers

[Mark Brown]

On Mon, Dec 05, 2011 at 08:37:23PM +0100, Pali Rohár wrote:

> I do not know regmap api, but I need interface which is easy readable from C 
> or shell scripts and which can read some registers (numbers 0-N).

The regmap debug file is a file of lines in the format 'regnum: value'
where both fields have a fixed width so should be easily parsable.

[PATCH 9/9] bq27x00: Fix OOPS caused by unregistring bq27x00 driver

[Pali Rohár]

* power_supply_unregister call bq27x00_battery_get_property which call bq27x00_battery_poll
* make sure that bq27x00_battery_poll will not call schedule_delayed_work again after unregister (which cause OOPS)

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq27x00_battery.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/power/bq27x00_battery.c b/drivers/power/bq27x00_battery.c
index 15ecd42..29ce907 100644
--- a/drivers/power/bq27x00_battery.c
+++ b/drivers/power/bq27x00_battery.c
@@ -722,6 +722,10 @@ static int bq27x00_powersupply_init(struct bq27x00_device_info *di)
 
 static void bq27x00_powersupply_unregister(struct bq27x00_device_info *di)
 {
+	/* power_supply_unregister call bq27x00_battery_get_property which call bq27x00_battery_poll */
+	/* make sure that bq27x00_battery_poll will not call schedule_delayed_work again after unregister (which cause OOPS) */
+	poll_interval = 0;
+
 	cancel_delayed_work_sync(&di->work);
 
 	bq27x00_battery_reg_exit(di);
-- 
1.7.5.4

Re: [PATCH 9/9] bq27x00: Fix OOPS caused by unregistring bq27x00 driver

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]

On Tuesday 01 November 2011 01:43:11 you wrote:
> * power_supply_unregister call bq27x00_battery_get_property which call
> bq27x00_battery_poll * make sure that bq27x00_battery_poll will not call
> schedule_delayed_work again after unregister (which cause OOPS)
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/power/bq27x00_battery.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/power/bq27x00_battery.c
> b/drivers/power/bq27x00_battery.c index 15ecd42..29ce907 100644
> --- a/drivers/power/bq27x00_battery.c
> +++ b/drivers/power/bq27x00_battery.c
> @@ -722,6 +722,10 @@ static int bq27x00_powersupply_init(struct
> bq27x00_device_info *di)
> 
>  static void bq27x00_powersupply_unregister(struct bq27x00_device_info *di)
>  {
> +	/* power_supply_unregister call bq27x00_battery_get_property which call
> bq27x00_battery_poll */ +	/* make sure that bq27x00_battery_poll will not
> call schedule_delayed_work again after unregister (which cause OOPS) */
> +	poll_interval = 0;
> +
>  	cancel_delayed_work_sync(&di->work);
> 
>  	bq27x00_battery_reg_exit(di);

Can somebody review this patch series? At least this last patch fix rebooting 
device Nokia N900 after unloading bq27x00_battery module with rmmod!

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH] rx51: add bq27200 i2c board info

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 1059 bytes --]

I forgot to send patch which enable bq27x00_battery driver on Nokia N900 RX51. Here is:

>From c06d00aa9d394ab5b65bbd5361c6da8424afbfe5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.rohar@gmail.com>
Date: Fri, 25 Nov 2011 23:42:54 +0100
Subject: [PATCH] rx51: add bq27200 i2c board info

---
 arch/arm/mach-omap2/board-rx51-peripherals.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c
index 5a886cd..23c626e 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -940,6 +940,11 @@ static struct i2c_board_info __initdata rx51_peripherals_i2c_board_info_2[] = {
 		.platform_data  = &rx51_lp5523_platform_data,
 	},
 #endif
+#if defined(CONFIG_BATTERY_BQ27X00_I2C) || defined(CONFIG_BATTERY_BQ27X00_I2C_MODULE)
+	{
+		I2C_BOARD_INFO("bq27200", 0x55),
+	},
+#endif
 	{
 		I2C_BOARD_INFO("tpa6130a2", 0x60),
 		.platform_data = &rx51_tpa6130a2_data,
-- 
1.7.5.4

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] rx51: add bq27200 i2c board info

[Anton Vorontsov]

Cc'ing Tony Lindgren.

Tony, OK to pass this via battery git tree?

Thanks,

p.s. Though, not sure if we really should bother w/ these #ifs.

On Fri, Nov 25, 2011 at 11:53:06PM +0100, Pali Rohár wrote:
> I forgot to send patch which enable bq27x00_battery driver on Nokia N900 RX51. Here is:
> 
> >From c06d00aa9d394ab5b65bbd5361c6da8424afbfe5 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.rohar@gmail.com>
> Date: Fri, 25 Nov 2011 23:42:54 +0100
> Subject: [PATCH] rx51: add bq27200 i2c board info
> 
> ---
>  arch/arm/mach-omap2/board-rx51-peripherals.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c
> index 5a886cd..23c626e 100644
> --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
> +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
> @@ -940,6 +940,11 @@ static struct i2c_board_info __initdata rx51_peripherals_i2c_board_info_2[] = {
>  		.platform_data  = &rx51_lp5523_platform_data,
>  	},
>  #endif
> +#if defined(CONFIG_BATTERY_BQ27X00_I2C) || defined(CONFIG_BATTERY_BQ27X00_I2C_MODULE)
> +	{
> +		I2C_BOARD_INFO("bq27200", 0x55),
> +	},
> +#endif
>  	{
>  		I2C_BOARD_INFO("tpa6130a2", 0x60),
>  		.platform_data = &rx51_tpa6130a2_data,

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

Re: [PATCH] rx51: add bq27200 i2c board info

[Felipe Contreras]

On Sat, Nov 26, 2011 at 1:03 AM, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
> Cc'ing Tony Lindgren.
>
> Tony, OK to pass this via battery git tree?

Tested-by: Felipe Contreras <felipe.contreras@nokia.com>

BTW. The current code has already plenty of #if's. I don't see
anything wrong with that.

-- 
Felipe Contreras

Re: [PATCH] rx51: add bq27200 i2c board info

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 1617 bytes --]

On Saturday 26 November 2011 03:03:21 Anton Vorontsov wrote:
> Cc'ing Tony Lindgren.
> 
> Tony, OK to pass this via battery git tree?
> 
> Thanks,
> 
> p.s. Though, not sure if we really should bother w/ these #ifs.
> 
> On Fri, Nov 25, 2011 at 11:53:06PM +0100, Pali Rohár wrote:
> > I forgot to send patch which enable bq27x00_battery driver on Nokia N900 
RX51. Here is:
> > >From c06d00aa9d394ab5b65bbd5361c6da8424afbfe5 Mon Sep 17 00:00:00 2001
> > 
> > From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.rohar@gmail.com>
> > Date: Fri, 25 Nov 2011 23:42:54 +0100
> > Subject: [PATCH] rx51: add bq27200 i2c board info
> > 
> > ---
> > 
> >  arch/arm/mach-omap2/board-rx51-peripherals.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
> > b/arch/arm/mach-omap2/board-rx51-peripherals.c index 5a886cd..23c626e
> > 100644
> > --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
> > +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
> > @@ -940,6 +940,11 @@ static struct i2c_board_info __initdata
> > rx51_peripherals_i2c_board_info_2[] = {> 
> >  		.platform_data  = &rx51_lp5523_platform_data,
> >  	
> >  	},
> >  
> >  #endif
> > 
> > +#if defined(CONFIG_BATTERY_BQ27X00_I2C) ||
> > defined(CONFIG_BATTERY_BQ27X00_I2C_MODULE) +	{
> > +		I2C_BOARD_INFO("bq27200", 0x55),
> > +	},
> > +#endif
> > 
> >  	{
> >  	
> >  		I2C_BOARD_INFO("tpa6130a2", 0x60),
> >  		.platform_data = &rx51_tpa6130a2_data,

What is problem with including this patch too?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] rx51: add bq27200 i2c board info

[Tony Lindgren]

* Anton Vorontsov <cbouatmailru@gmail.com> [111125 14:28]:
> Cc'ing Tony Lindgren.
> 
> Tony, OK to pass this via battery git tree?

Sorry for the delay, yes that's fine with me:

Acked-by: Tony Lindgren <tony@atomide.com>
 
> p.s. Though, not sure if we really should bother w/ these #ifs.

Up to you, looks like they could be left out.
 
> On Fri, Nov 25, 2011 at 11:53:06PM +0100, Pali Rohár wrote:
> > I forgot to send patch which enable bq27x00_battery driver on Nokia N900 RX51. Here is:
> > 
> > >From c06d00aa9d394ab5b65bbd5361c6da8424afbfe5 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.rohar@gmail.com>
> > Date: Fri, 25 Nov 2011 23:42:54 +0100
> > Subject: [PATCH] rx51: add bq27200 i2c board info
> > 
> > ---
> >  arch/arm/mach-omap2/board-rx51-peripherals.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c
> > index 5a886cd..23c626e 100644
> > --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
> > +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
> > @@ -940,6 +940,11 @@ static struct i2c_board_info __initdata rx51_peripherals_i2c_board_info_2[] = {
> >  		.platform_data  = &rx51_lp5523_platform_data,
> >  	},
> >  #endif
> > +#if defined(CONFIG_BATTERY_BQ27X00_I2C) || defined(CONFIG_BATTERY_BQ27X00_I2C_MODULE)
> > +	{
> > +		I2C_BOARD_INFO("bq27200", 0x55),
> > +	},
> > +#endif
> >  	{
> >  		I2C_BOARD_INFO("tpa6130a2", 0x60),
> >  		.platform_data = &rx51_tpa6130a2_data,
> 
> -- 
> Anton Vorontsov
> Email: cbouatmailru@gmail.com

Re: [PATCH] rx51: add bq27200 i2c board info

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 1967 bytes --]

On Wednesday 07 December 2011 12:25:47 you wrote:
> * Anton Vorontsov <cbouatmailru@gmail.com> [111125 14:28]:
> > Cc'ing Tony Lindgren.
> > 
> > Tony, OK to pass this via battery git tree?
> 
> Sorry for the delay, yes that's fine with me:
> 
> Acked-by: Tony Lindgren <tony@atomide.com>

Anton Vorontsov, can you now include this patch into battery tree?

> 
> > p.s. Though, not sure if we really should bother w/ these #ifs.
> 
> Up to you, looks like they could be left out.

Now I see that these #ifs is not needed.

> 
> > On Fri, Nov 25, 2011 at 11:53:06PM +0100, Pali Rohár wrote:
> > > I forgot to send patch which enable bq27x00_battery driver on Nokia N900 
RX51. Here is:
> > > >From c06d00aa9d394ab5b65bbd5361c6da8424afbfe5 Mon Sep 17 00:00:00
> > > >2001
> > > 
> > > From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.rohar@gmail.com>
> > > Date: Fri, 25 Nov 2011 23:42:54 +0100
> > > Subject: [PATCH] rx51: add bq27200 i2c board info
> > > 
> > > ---
> > > 
> > >  arch/arm/mach-omap2/board-rx51-peripherals.c |    5 +++++
> > >  1 files changed, 5 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
> > > b/arch/arm/mach-omap2/board-rx51-peripherals.c index
> > > 5a886cd..23c626e 100644
> > > --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
> > > +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
> > > @@ -940,6 +940,11 @@ static struct i2c_board_info __initdata
> > > rx51_peripherals_i2c_board_info_2[] = {> > 
> > >  		.platform_data  = &rx51_lp5523_platform_data,
> > >  	
> > >  	},
> > >  
> > >  #endif
> > > 
> > > +#if defined(CONFIG_BATTERY_BQ27X00_I2C) ||
> > > defined(CONFIG_BATTERY_BQ27X00_I2C_MODULE) +	{
> > > +		I2C_BOARD_INFO("bq27200", 0x55),
> > > +	},
> > > +#endif
> > > 
> > >  	{
> > >  	
> > >  		I2C_BOARD_INFO("tpa6130a2", 0x60),
> > >  		.platform_data = &rx51_tpa6130a2_data,

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] rx51: add bq27200 i2c board info

[Anton Vorontsov]

On Wed, Dec 07, 2011 at 12:25:47PM -0800, Tony Lindgren wrote:
> * Anton Vorontsov <cbouatmailru@gmail.com> [111125 14:28]:
> > Cc'ing Tony Lindgren.
> > 
> > Tony, OK to pass this via battery git tree?
> 
> Sorry for the delay, yes that's fine with me:
> 
> Acked-by: Tony Lindgren <tony@atomide.com>

Applied, thanks!

p.s. Pali, you did not provide a sign-off for this patch. The patch is
trivial enough, so I took the risk to apply it w/ only my sign-off. But
please, provide yours here for the history. Just to be safe. :-)

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com