Hi

this is a revision. I prefer to change the function instead use a wrapper.
Is ok Werner?

Michael
Protect the data using a mutex. Fix a race that can happen when
the user read from the sysfs and the worker execute in the middle.

Signed-off-by: Michael Trimarchi <[email protected]>

---
diff --git a/drivers/power/bq27000_battery.c b/drivers/power/bq27000_battery.c
index 01168ea..24f0c31 100644
--- a/drivers/power/bq27000_battery.c
+++ b/drivers/power/bq27000_battery.c
@@ -134,6 +134,7 @@ struct bq27000_device_info {
 	struct bq27000_bat_regs regs;
 };
 
+static DEFINE_MUTEX(battery_mutex);
 static unsigned int cache_time = 5000;
 module_param(cache_time, uint, 0644);
 MODULE_PARM_DESC(cache_time, "cache time in milliseconds");
@@ -210,11 +211,15 @@ static int bq27000_battery_get_property(struct power_supply *psy,
 				       union power_supply_propval *val)
 {
 	int n;
+	int ret = 0;
 	struct bq27000_device_info *di =
 		container_of(psy, struct bq27000_device_info, bat);
 
-	if (di->regs.rsoc < 0 && psp != POWER_SUPPLY_PROP_PRESENT)
-		return -ENODEV;
+	mutex_lock(&battery_mutex);
+	if (di->regs.rsoc < 0 && psp != POWER_SUPPLY_PROP_PRESENT) {
+		ret = -ENODEV;
+		goto out_unlock;
+	}
 
 	switch (psp) {
 	case POWER_SUPPLY_PROP_STATUS:
@@ -265,8 +270,10 @@ use_bat:
 			break;
 		}
 		/* power is actually going in or out... */
-		if (di->regs.flags < 0)
-			return di->regs.flags;
+		if (di->regs.flags < 0) {
+			ret = di->regs.flags;
+			goto out_unlock;
+		}
 		if (di->regs.flags & BQ27000_STATUS_CHGS)
 			val->intval = POWER_SUPPLY_STATUS_CHARGING;
 		else
@@ -275,41 +282,55 @@ use_bat:
 	case POWER_SUPPLY_PROP_HEALTH:
 		val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
 		/* Do we have accurate readings... */
-		if (di->regs.flags < 0)
-			return di->regs.flags;
+		if (di->regs.flags < 0) {
+			ret = di->regs.flags;
+			goto out_unlock;
+		}
 		if (di->regs.flags & BQ27000_STATUS_VDQ)
 			val->intval = POWER_SUPPLY_HEALTH_GOOD;
 		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
-		if (di->regs.volt < 0)
-			return di->regs.volt;
+		if (di->regs.volt < 0) {
+			ret = di->regs.volt;
+			goto out_unlock;
+		}
 		/* mV -> uV */
 		val->intval = di->regs.volt * 1000;
 		break;
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
-		if (di->regs.flags < 0)
-			return di->regs.flags;
+		if (di->regs.flags < 0) {
+			ret = di->regs.flags;
+			goto out_unlock;
+		}
 		if (di->regs.flags & BQ27000_STATUS_CHGS)
 			n = -NANOVOLTS_UNIT;
 		else
 			n = NANOVOLTS_UNIT;
-		if (di->regs.ai < 0)
-			return di->regs.ai;
+		if (di->regs.ai < 0) {
+			ret = di->regs.ai;
+			goto out_unlock;
+		}
 		val->intval = (di->regs.ai * n) / di->pdata->rsense_mohms;
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL:
-		if (di->regs.lmd < 0)
-			return di->regs.lmd;
+		if (di->regs.lmd < 0) {
+			ret = di->regs.lmd;
+			goto out_unlock;
+		}
 		val->intval = (di->regs.lmd * 3570) / di->pdata->rsense_mohms;
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_NOW:
-		if (di->regs.nac < 0)
-			return di->regs.nac;
+		if (di->regs.nac < 0) {
+			ret = di->regs.nac;
+			goto out_unlock;
+		}
 		val->intval = (di->regs.nac * 3570) / di->pdata->rsense_mohms;
 		break;
 	case POWER_SUPPLY_PROP_TEMP:
-		if (di->regs.temp < 0)
-			return di->regs.temp;
+		if (di->regs.temp < 0) {
+			ret = di->regs.temp;
+			goto out_unlock;
+		}
 		/* K (in 0.25K units) is 273.15 up from C (in 0.1C)*/
 		/* 10926 = 27315 * 4 / 10 */
 		val->intval = (((long)di->regs.temp * 10l) - 10926) / 4;
@@ -319,33 +340,44 @@ use_bat:
 		break;
 	case POWER_SUPPLY_PROP_CAPACITY:
 		val->intval = di->regs.rsoc;
-		if (val->intval < 0)
-			return val->intval;
+		if (val->intval < 0) {
+			ret = val->intval;
+			goto out_unlock;
+		}
 		break;
 	case POWER_SUPPLY_PROP_PRESENT:
 		val->intval = !(di->regs.rsoc < 0);
 		break;
 	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
-		if (di->regs.tte < 0)
-			return di->regs.tte;
+		if (di->regs.tte < 0) {
+			ret = di->regs.tte;
+			goto out_unlock;
+		}
 		val->intval = 60 * di->regs.tte;
 		break;
 	case POWER_SUPPLY_PROP_TIME_TO_FULL_NOW:
-		if (di->regs.ttf < 0)
-			return di->regs.ttf;
+		if (di->regs.ttf < 0) {
+			ret = di->regs.ttf;
+			goto out_unlock;
+		}
 		val->intval = 60 * di->regs.ttf;
 		break;
 	case POWER_SUPPLY_PROP_ONLINE:
 		if (di->pdata->get_charger_online_status)
 			val->intval = (di->pdata->get_charger_online_status)();
-		else
-			return -EINVAL;
+		else {
+			ret = -EINVAL;
+			goto out_unlock;
+		}
 		break;
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
+		break;
 	}
 
-	return 0;
+out_unlock:
+	mutex_unlock(&battery_mutex);
+	return ret;
 }
 
 static void bq27000_battery_work(struct work_struct *work)
@@ -368,7 +400,9 @@ static void bq27000_battery_work(struct work_struct *work)
 		update_hdq_data(di, BQ27000_RSOC, &regs.rsoc);
 
 		if (memcmp (&regs, &di->regs, sizeof(regs)) != 0) {
+			mutex_lock(&battery_mutex);
 			di->regs = regs;
+			mutex_unlock(&battery_mutex);
 			power_supply_changed(&di->bat);
 		}
 	}

Reply via email to