[PATCH 0/3] Update charger manager driver to support device tree

[PATCH 0/3] Update charger manager driver to support device tree

[Jonghwa Lee]

Until now, charger manager doesn't support DT at all. It only works with
legacy platform data. Therefore, charger_desc structure which is requisite
data for working contains non-friendly variables for DT, as like funtion ptr,
and u64 type data.

These patches modifies charger_desc structure and driver to support device tree
fully. For using device tree in charger manager, you can check example of
charger manager's device tree model in Documentaion directory. 

Currently, there're many required nodes for proper charger manager's
working, we would optimize them later.

Jonghwa Lee (3):
  charger-manager : Replace kzalloc to devm_kzalloc and remove
    uneccessary code.
  charger-manager : Add default battery temperature checking funtion.
  charger-manager : Support deivce tree in charger manager driver

 .../bindings/power_supply/charger-manager.txt      |   82 +++++
 drivers/power/charger-manager.c                    |  315 +++++++++++++++-----
 include/linux/power/charger-manager.h              |   21 +-
 3 files changed, 340 insertions(+), 78 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power_supply/charger-manager.txt

-- 
1.7.9.5

[PATCH 1/3] charger-manager : Replace kzalloc to devm_kzalloc and remove uneccessary code.

[Jonghwa Lee]

Use devm function for dynamic memory allocation.

Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
---
 drivers/power/charger-manager.c |   85 +++++++++++++--------------------------
 1 file changed, 27 insertions(+), 58 deletions(-)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index e30e847..7287c0e 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -1378,7 +1378,8 @@ static int charger_manager_register_sysfs(struct charger_manager *cm)
 		charger = &desc->charger_regulators[i];
 
 		snprintf(buf, 10, "charger.%d", i);
-		str = kzalloc(sizeof(char) * (strlen(buf) + 1), GFP_KERNEL);
+		str = devm_kzalloc(cm->dev,
+				sizeof(char) * (strlen(buf) + 1), GFP_KERNEL);
 		if (!str) {
 			ret = -ENOMEM;
 			goto err;
@@ -1452,30 +1453,23 @@ static int charger_manager_probe(struct platform_device *pdev)
 			rtc_dev = NULL;
 			dev_err(&pdev->dev, "Cannot get RTC %s\n",
 				g_desc->rtc_name);
-			ret = -ENODEV;
-			goto err_alloc;
+			return -ENODEV;
 		}
 	}
 
 	if (!desc) {
 		dev_err(&pdev->dev, "No platform data (desc) found\n");
-		ret = -ENODEV;
-		goto err_alloc;
+		return -ENODEV;
 	}
 
-	cm = kzalloc(sizeof(struct charger_manager), GFP_KERNEL);
-	if (!cm) {
-		ret = -ENOMEM;
-		goto err_alloc;
-	}
+	cm = devm_kzalloc(&pdev->dev,
+			sizeof(struct charger_manager),	GFP_KERNEL);
+	if (!cm)
+		return -ENOMEM;
 
 	/* Basic Values. Unspecified are Null or 0 */
 	cm->dev = &pdev->dev;
-	cm->desc = kmemdup(desc, sizeof(struct charger_desc), GFP_KERNEL);
-	if (!cm->desc) {
-		ret = -ENOMEM;
-		goto err_alloc_desc;
-	}
+	cm->desc = desc;
 	cm->last_temp_mC = INT_MIN; /* denotes "unmeasured, yet" */
 
 	/*
@@ -1498,27 +1492,23 @@ static int charger_manager_probe(struct platform_device *pdev)
 	}
 
 	if (!desc->charger_regulators || desc->num_charger_regulators < 1) {
-		ret = -EINVAL;
 		dev_err(&pdev->dev, "charger_regulators undefined\n");
-		goto err_no_charger;
+		return -EINVAL;
 	}
 
 	if (!desc->psy_charger_stat || !desc->psy_charger_stat[0]) {
 		dev_err(&pdev->dev, "No power supply defined\n");
-		ret = -EINVAL;
-		goto err_no_charger_stat;
+		return -EINVAL;
 	}
 
 	/* Counting index only */
 	while (desc->psy_charger_stat[i])
 		i++;
 
-	cm->charger_stat = kzalloc(sizeof(struct power_supply *) * (i + 1),
-				   GFP_KERNEL);
-	if (!cm->charger_stat) {
-		ret = -ENOMEM;
-		goto err_no_charger_stat;
-	}
+	cm->charger_stat = devm_kzalloc(&pdev->dev,
+				sizeof(struct power_supply *) * i, GFP_KERNEL);
+	if (!cm->charger_stat)
+		return -ENOMEM;
 
 	for (i = 0; desc->psy_charger_stat[i]; i++) {
 		cm->charger_stat[i] = power_supply_get_by_name(
@@ -1526,8 +1516,7 @@ static int charger_manager_probe(struct platform_device *pdev)
 		if (!cm->charger_stat[i]) {
 			dev_err(&pdev->dev, "Cannot find power supply \"%s\"\n",
 				desc->psy_charger_stat[i]);
-			ret = -ENODEV;
-			goto err_chg_stat;
+			return -ENODEV;
 		}
 	}
 
@@ -1535,21 +1524,18 @@ static int charger_manager_probe(struct platform_device *pdev)
 	if (!cm->fuel_gauge) {
 		dev_err(&pdev->dev, "Cannot find power supply \"%s\"\n",
 			desc->psy_fuel_gauge);
-		ret = -ENODEV;
-		goto err_chg_stat;
+		return -ENODEV;
 	}
 
 	if (desc->polling_interval_ms == 0 ||
 	    msecs_to_jiffies(desc->polling_interval_ms) <= CM_JIFFIES_SMALL) {
 		dev_err(&pdev->dev, "polling_interval_ms is too small\n");
-		ret = -EINVAL;
-		goto err_chg_stat;
+		return -EINVAL;
 	}
 
 	if (!desc->temperature_out_of_range) {
 		dev_err(&pdev->dev, "there is no temperature_out_of_range\n");
-		ret = -EINVAL;
-		goto err_chg_stat;
+		return -EINVAL;
 	}
 
 	if (!desc->charging_max_duration_ms ||
@@ -1570,14 +1556,13 @@ static int charger_manager_probe(struct platform_device *pdev)
 	cm->charger_psy.name = cm->psy_name_buf;
 
 	/* Allocate for psy properties because they may vary */
-	cm->charger_psy.properties = kzalloc(sizeof(enum power_supply_property)
+	cm->charger_psy.properties = devm_kzalloc(&pdev->dev,
+				sizeof(enum power_supply_property)
 				* (ARRAY_SIZE(default_charger_props) +
-				NUM_CHARGER_PSY_OPTIONAL),
-				GFP_KERNEL);
-	if (!cm->charger_psy.properties) {
-		ret = -ENOMEM;
-		goto err_chg_stat;
-	}
+				NUM_CHARGER_PSY_OPTIONAL), GFP_KERNEL);
+	if (!cm->charger_psy.properties)
+		return -ENOMEM;
+
 	memcpy(cm->charger_psy.properties, default_charger_props,
 		sizeof(enum power_supply_property) *
 		ARRAY_SIZE(default_charger_props));
@@ -1614,7 +1599,7 @@ static int charger_manager_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(&pdev->dev, "Cannot register charger-manager with name \"%s\"\n",
 			cm->charger_psy.name);
-		goto err_register;
+		return ret;
 	}
 
 	/* Register extcon device for charger cable */
@@ -1655,8 +1640,6 @@ err_reg_sysfs:
 		charger = &desc->charger_regulators[i];
 		sysfs_remove_group(&cm->charger_psy.dev->kobj,
 				&charger->attr_g);
-
-		kfree(charger->attr_g.name);
 	}
 err_reg_extcon:
 	for (i = 0; i < desc->num_charger_regulators; i++) {
@@ -1674,16 +1657,7 @@ err_reg_extcon:
 	}
 
 	power_supply_unregister(&cm->charger_psy);
-err_register:
-	kfree(cm->charger_psy.properties);
-err_chg_stat:
-	kfree(cm->charger_stat);
-err_no_charger_stat:
-err_no_charger:
-	kfree(cm->desc);
-err_alloc_desc:
-	kfree(cm);
-err_alloc:
+
 	return ret;
 }
 
@@ -1718,11 +1692,6 @@ static int charger_manager_remove(struct platform_device *pdev)
 
 	try_charger_enable(cm, false);
 
-	kfree(cm->charger_psy.properties);
-	kfree(cm->charger_stat);
-	kfree(cm->desc);
-	kfree(cm);
-
 	return 0;
 }
 
-- 
1.7.9.5

[PATCH 2/3] charger-manager : Add default battery temperature checking funtion.

[Jonghwa Lee]

During the charger manager driver's probing time, it can't succeed
if there's no pre-defined .temperature_out_of_range callback function.
But if fuel gauge supports battery temperature measurement, we
can use it directly. That's what cm_default_get_temp() function does.

With flag measure_batter_temp ON, we normally use cm_default_get_temp()
for .temperature_out_of_range callback funtion.
The TEMP_AMBIENT property is only used for pre-defined one.

Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
---
 drivers/power/charger-manager.c       |   88 ++++++++++++++++++++++++++++-----
 include/linux/power/charger-manager.h |   11 ++++-
 2 files changed, 86 insertions(+), 13 deletions(-)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 7287c0e..959eace 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -550,7 +550,7 @@ static int check_charging_duration(struct charger_manager *cm)
 static bool _cm_monitor(struct charger_manager *cm)
 {
 	struct charger_desc *desc = cm->desc;
-	int temp = desc->temperature_out_of_range(&cm->last_temp_mC);
+	int temp = desc->temperature_out_of_range(cm);
 
 	dev_dbg(cm->dev, "monitoring (%2.2d.%3.3dC)\n",
 		cm->last_temp_mC / 1000, cm->last_temp_mC % 1000);
@@ -802,18 +802,16 @@ static int charger_get_property(struct power_supply *psy,
 				POWER_SUPPLY_PROP_CURRENT_NOW, val);
 		break;
 	case POWER_SUPPLY_PROP_TEMP:
-		/* in thenth of centigrade */
 		if (cm->last_temp_mC == INT_MIN)
-			desc->temperature_out_of_range(&cm->last_temp_mC);
-		val->intval = cm->last_temp_mC / 100;
+			desc->temperature_out_of_range(cm);
+		val->intval = cm->last_temp_mC;
 		if (!desc->measure_battery_temp)
 			ret = -ENODEV;
 		break;
 	case POWER_SUPPLY_PROP_TEMP_AMBIENT:
-		/* in thenth of centigrade */
 		if (cm->last_temp_mC == INT_MIN)
-			desc->temperature_out_of_range(&cm->last_temp_mC);
-		val->intval = cm->last_temp_mC / 100;
+			desc->temperature_out_of_range(cm);
+		val->intval = cm->last_temp_mC;
 		if (desc->measure_battery_temp)
 			ret = -ENODEV;
 		break;
@@ -1439,6 +1437,70 @@ err:
 	return ret;
 }
 
+/* Every temperature units are in milli centigrade */
+#define CM_DEFAULT_TEMP_ALERT_DIFF	10000
+#define CM_DEFAULT_TEMP_ALERT_MAX	127000
+#define CM_DEFAULT_TEMP_ALERT_MIN	(-127000)
+
+static int cm_default_get_temp(struct charger_manager *cm)
+{
+	struct charger_desc *desc = cm->desc;
+	union power_supply_propval val;
+	static int temp_alert_min = 0;
+	static int temp_alert_max = 0;
+	static int temp_alert_diff = 0;
+	static int last_temp_status = 0;
+	int ret;
+
+	if (!temp_alert_min && !temp_alert_max) {
+		/* Initialize minimum temperature for alert */
+		ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
+					POWER_SUPPLY_PROP_TEMP_ALERT_MIN,
+					&val);
+		temp_alert_min = ret ? desc->temp_alert_min :
+					min(desc->temp_alert_min, val.intval);
+		if (!temp_alert_min)
+			temp_alert_min = CM_DEFAULT_TEMP_ALERT_MIN;
+
+		/* Initialize maximum temperature for alert */
+		ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
+					POWER_SUPPLY_PROP_TEMP_ALERT_MAX,
+					&val);
+		temp_alert_max = ret ? desc->temp_alert_max :
+					min(desc->temp_alert_max, val.intval);
+		if (!temp_alert_max)
+			temp_alert_max = CM_DEFAULT_TEMP_ALERT_MAX;
+
+		temp_alert_diff = desc->temp_alert_diff ?
+					: CM_DEFAULT_TEMP_ALERT_DIFF;
+	}
+
+	/* Get battery temperature */
+	ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
+					POWER_SUPPLY_PROP_TEMP,
+					&val);
+	if (ret) {
+		cm->last_temp_mC = INT_MIN;
+		return 0;
+	}
+
+	cm->last_temp_mC = val.intval;
+
+	if (cm->last_temp_mC > temp_alert_max || (last_temp_status &&
+			cm->last_temp_mC + temp_alert_diff > temp_alert_max)) {
+		/* OVERHEAT */
+		last_temp_status = 1;
+	} else if (cm->last_temp_mC < temp_alert_min ||	(last_temp_status &&
+			cm->last_temp_mC < temp_alert_min + temp_alert_diff)) {
+		/* Too COLD */
+		last_temp_status = -1;
+	} else {
+		last_temp_status = 0;
+	}
+
+	return last_temp_status;
+}
+
 static int charger_manager_probe(struct platform_device *pdev)
 {
 	struct charger_desc *desc = dev_get_platdata(&pdev->dev);
@@ -1533,11 +1595,6 @@ static int charger_manager_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	if (!desc->temperature_out_of_range) {
-		dev_err(&pdev->dev, "there is no temperature_out_of_range\n");
-		return -EINVAL;
-	}
-
 	if (!desc->charging_max_duration_ms ||
 			!desc->discharging_max_duration_ms) {
 		dev_info(&pdev->dev, "Cannot limit charging duration checking mechanism to prevent overcharge/overheat and control discharging duration\n");
@@ -1587,7 +1644,14 @@ static int charger_manager_probe(struct platform_device *pdev)
 		cm->charger_psy.properties[cm->charger_psy.num_properties] =
 				POWER_SUPPLY_PROP_TEMP;
 		cm->charger_psy.num_properties++;
+		desc->temperature_out_of_range = cm_default_get_temp;
 	} else {
+		if (!desc->temperature_out_of_range) {
+			dev_err(&pdev->dev,
+				"%s : No battery thermometer exists\n",
+				__func__);
+			return -EINVAL;
+		}
 		cm->charger_psy.properties[cm->charger_psy.num_properties] =
 				POWER_SUPPLY_PROP_TEMP_AMBIENT;
 		cm->charger_psy.num_properties++;
diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
index 0e86840..2088619 100644
--- a/include/linux/power/charger-manager.h
+++ b/include/linux/power/charger-manager.h
@@ -148,6 +148,8 @@ struct charger_regulator {
 	struct charger_manager *cm;
 };
 
+struct charger_manager;
+
 /**
  * struct charger_desc
  * @psy_name: the name of power-supply-class for charger manager
@@ -173,6 +175,9 @@ struct charger_regulator {
  * @num_charger_regulator: the number of entries in charger_regulators
  * @charger_regulators: array of charger regulators
  * @psy_fuel_gauge: the name of power-supply for fuel gauge
+ * @temp_alert_min : Minimum battery temperature to be allowed.
+ * @temp_alert_max : Maximum battery temperature to be allowed.
+ * @temp_alert_diff : Temperature diffential to restart charging. 
  * @temperature_out_of_range:
  *	Determine whether the status is overheat or cold or normal.
  *	return_value > 0: overheat
@@ -210,7 +215,11 @@ struct charger_desc {
 
 	char *psy_fuel_gauge;
 
-	int (*temperature_out_of_range)(int *mC);
+	int temp_alert_min;
+	int temp_alert_max;
+	int temp_alert_diff;
+
+	int (*temperature_out_of_range)(struct charger_manager *);
 	bool measure_battery_temp;
 
 	u64 charging_max_duration_ms;
-- 
1.7.9.5

[PATCH 3/3] charger-manager : Support deivce tree in charger manager driver

[Jonghwa Lee]

With this patch, charger manager can parse charger_desc data from DT
which is used to register charger manager.

Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
---
 .../bindings/power_supply/charger-manager.txt      |   82 +++++++++++
 drivers/power/charger-manager.c                    |  144 +++++++++++++++++++-
 include/linux/power/charger-manager.h              |   10 +-
 3 files changed, 228 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power_supply/charger-manager.txt

diff --git a/Documentation/devicetree/bindings/power_supply/charger-manager.txt b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
new file mode 100644
index 0000000..806ea67
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
@@ -0,0 +1,82 @@
+charger-manager bindings
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+Required properties :
+ - compatible : "charger-manager"
+ - <>-supply : for regulator consumer
+ - cm-num-chargers : number of chargers
+ - cm-chargers : name of chargers
+ - subnode <regulator> :
+	- cm-regulator-name : name of charger regulator
+	- subnode <cable> :
+		- cm-cable-name : name of charger cable
+		- cm-cable-extcon : name of extcon dev
+(optional)	- cm-cable-min : minimum current of cable
+(optional)	- cm-cable-max : maximum current of cable
+
+Optional properties :
+ - cm-name : charger manager's name (default : "battery")
+ - cm-poll-mode : polling mode (enum polling_modes)
+ - cm-poll-interval : polling interval
+ - cm-battery-stat : battery status (enum data_source)
+ - cm-fullbatt-* : data for full battery checking
+ - cm-battery-* : data related to battery temperature
+	-cold : critical cold temperature for battery
+	-in-minus : flag that cold temerature is in minus degree
+	-hot : critical hot temperature for battery
+	-temp-diff : temperature difference to allow recharging
+ - cm-battery-has-therm : flag that fuel gauge has thermometer
+ - cm-dis/charging-max = limits of charging duration
+
+Example :
+	charger-manager@0 {
+		compatible = "charger-manager";
+		chg-reg-supply = <&charger_regulator>;
+
+		cm-name = "battery";
+		/* Always polling ON : 30s */
+		cm-poll-mode = <1>;
+		cm-poll-interval = <30000>;
+
+		cm-fullbatt-vchkdrop-ms = <30000>;
+		cm-fullbatt-vchkdrop-volt = <150000>;
+		cm-fullbatt-soc = <100>;
+
+		cm-battery-stat = <3>;
+
+		/* in milli centigrade */
+		cm-battery-cold = <5000>;
+		cm-battery-cold-in-minus;
+		cm-battery-hot = <80000>;
+		cm-battery-temp-diff = <15000>;
+
+		cm-num-chargers = <3>;
+		cm-chargers = "charger0", "charger1", "charger2";
+
+		cm-fuel-gauge = "fuelgauge0";
+		cm-battery-has-therm;
+
+		/* Allow charging for 5hr */
+		cm-charging-max = <18000000>;
+		/* Allow discharging for 2hr */
+		cm-discharging-max = <7200000>;
+
+		regulator@0 {
+			cm-regulator-name = "chg-reg";
+			cable@0 {
+				cm-cable-name = "USB";
+				cm-cable-extcon = "extcon-dev.0";
+				cm-cable-min = <475000>;
+				cm-cable-max = <500000>;
+			};
+			cable@1 {
+				cm-cable-name = "TA";
+				cm-cable-extcon = "extcon-dev.0";
+				cm-cable-min = <650000>;
+				cm-cable-max = <675000>;
+			};
+		};
+
+	};
+
+
diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 959eace..8576675 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -518,7 +518,7 @@ static int check_charging_duration(struct charger_manager *cm)
 		duration = curr - cm->charging_start_time;
 
 		if (duration > desc->charging_max_duration_ms) {
-			dev_info(cm->dev, "Charging duration exceed %lldms\n",
+			dev_info(cm->dev, "Charging duration exceed %ums\n",
 				 desc->charging_max_duration_ms);
 			uevent_notify(cm, "Discharging");
 			try_charger_enable(cm, false);
@@ -529,7 +529,7 @@ static int check_charging_duration(struct charger_manager *cm)
 
 		if (duration > desc->charging_max_duration_ms &&
 				is_ext_pwr_online(cm)) {
-			dev_info(cm->dev, "Discharging duration exceed %lldms\n",
+			dev_info(cm->dev, "Discharging duration exceed %ums\n",
 				 desc->discharging_max_duration_ms);
 			uevent_notify(cm, "Recharging");
 			try_charger_enable(cm, true);
@@ -1501,9 +1501,146 @@ static int cm_default_get_temp(struct charger_manager *cm)
 	return last_temp_status;
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id charger_manager_match[] = {
+	{
+		.compatible = "charger-manager",
+	},
+	{},
+ };
+
+struct charger_desc *of_cm_parse_desc(struct device *dev)
+{
+	struct charger_desc *desc;
+	struct device_node *np = dev->of_node;
+	u32 poll_mode = CM_POLL_DISABLE;
+	u32 battery_stat = CM_NO_BATTERY;
+	int num_chgs;
+
+	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
+	if (!desc)
+		return ERR_PTR(-ENOMEM);
+
+	of_property_read_string(np, "cm-name", &desc->psy_name);
+
+	of_property_read_u32(np, "cm-poll-mode", &poll_mode);
+	desc->polling_mode = poll_mode;
+
+	of_property_read_u32(np, "cm-poll-interval",
+				&desc->polling_interval_ms);
+
+	of_property_read_u32(np, "cm-fullbatt-vchkdrop-ms",
+					&desc->fullbatt_vchkdrop_ms);
+	of_property_read_u32(np, "cm-fullbatt-vchkdrop-volt",
+					&desc->fullbatt_vchkdrop_uV);
+	of_property_read_u32(np, "cm-fullbatt-voltage", &desc->fullbatt_uV);
+	of_property_read_u32(np, "cm-fullbatt-soc", &desc->fullbatt_soc);
+	of_property_read_u32(np, "cm-fullbatt-capacity",
+					&desc->fullbatt_full_capacity);
+
+	of_property_read_u32(np, "cm-battery-stat", &battery_stat);
+	desc->battery_present = battery_stat;
+
+	of_property_read_u32(np, "cm-battery-cold", &desc->temp_alert_min);
+	if (of_get_property(np, "cm-battery-cold-in-minus", NULL))
+		desc->temp_alert_min *= -1;
+	of_property_read_u32(np, "cm-battery-hot", &desc->temp_alert_max);
+	of_property_read_u32(np, "cm-battery-temp-diff",
+					&desc->temp_alert_diff);
+
+	/* chargers */
+	of_property_read_u32(np, "cm-num-chargers", &num_chgs);
+	if (num_chgs) {
+		/* Allocate empty bin at the tail of array */
+		desc->psy_charger_stat = devm_kzalloc(dev, sizeof(char *)
+						* (num_chgs + 1), GFP_KERNEL);
+		if (desc->psy_charger_stat) {
+			int i;
+			for (i = 0; i < num_chgs; i++)
+				of_property_read_string_index(np, "cm-chargers",
+						i, &desc->psy_charger_stat[i]);
+		} else {
+			return ERR_PTR(-ENOMEM);
+		}
+	}
+
+	of_property_read_string(np, "cm-fuel-gauge", &desc->psy_fuel_gauge);
+	if (of_get_property(np, "cm-battery-has-therm", NULL))
+		desc->measure_battery_temp = true;
+
+	of_property_read_u32(np, "cm-charging-max",
+				&desc->charging_max_duration_ms);
+	of_property_read_u32(np, "cm-discharging-max",
+				&desc->discharging_max_duration_ms);
+
+	/* battery charger regualtors */
+	desc->num_charger_regulators = of_get_child_count(np);
+	if (desc->num_charger_regulators) {
+		struct charger_regulator *chg_regs;
+		struct device_node *child;
+
+		chg_regs = devm_kzalloc(dev, sizeof(*chg_regs)
+					* desc->num_charger_regulators,
+					GFP_KERNEL);
+		if (!chg_regs)
+			return ERR_PTR(-ENOMEM);
+
+		desc->charger_regulators = chg_regs;
+
+		for_each_child_of_node(np, child) {
+			struct charger_cable *cables;
+			struct device_node *_child;
+
+			of_property_read_string(child, "cm-regulator-name",
+					&chg_regs->regulator_name);
+
+			/* charger cables */
+			chg_regs->num_cables = of_get_child_count(child);
+			if (chg_regs->num_cables) {
+				cables = devm_kzalloc(dev, sizeof(*cables)
+						* chg_regs->num_cables,
+						GFP_KERNEL);
+				if (!cables)
+					return ERR_PTR(-ENOMEM);
+
+				chg_regs->cables = cables;
+
+				for_each_child_of_node(child, _child) {
+					of_property_read_string(_child,
+					"cm-cable-name", &cables->name);
+					of_property_read_string(_child,
+					"cm-cable-extcon",
+					&cables->extcon_name);
+					of_property_read_u32(_child,
+					"cm-cable-min",
+					&cables->min_uA);
+					of_property_read_u32(_child,
+					"cm-cable-max",
+					&cables->max_uA);
+					cables++;
+				}
+			}
+			chg_regs++;
+		}
+	}
+	return desc;
+}
+#else
+#define charger_manager_match	NULL
+#endif
+
+static inline struct charger_desc *cm_get_drv_data(struct platform_device *pdev)
+{
+#ifdef CONFIG_OF
+	if (pdev->dev.of_node)
+		return of_cm_parse_desc(&pdev->dev);
+#endif
+	return (struct charger_desc *)dev_get_platdata(&pdev->dev);
+}
+
 static int charger_manager_probe(struct platform_device *pdev)
 {
-	struct charger_desc *desc = dev_get_platdata(&pdev->dev);
+	struct charger_desc *desc = cm_get_drv_data(pdev);
 	struct charger_manager *cm;
 	int ret = 0, i = 0;
 	int j = 0;
@@ -1872,6 +2009,7 @@ static struct platform_driver charger_manager_driver = {
 		.name = "charger-manager",
 		.owner = THIS_MODULE,
 		.pm = &charger_manager_pm,
+		.of_match_table = charger_manager_match,
 	},
 	.probe = charger_manager_probe,
 	.remove = charger_manager_remove,
diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
index 2088619..0f6f574 100644
--- a/include/linux/power/charger-manager.h
+++ b/include/linux/power/charger-manager.h
@@ -195,7 +195,7 @@ struct charger_manager;
  *	max_duration_ms', cm start charging.
  */
 struct charger_desc {
-	char *psy_name;
+	const char *psy_name;
 
 	enum polling_modes polling_mode;
 	unsigned int polling_interval_ms;
@@ -208,12 +208,12 @@ struct charger_desc {
 
 	enum data_source battery_present;
 
-	char **psy_charger_stat;
+	const char **psy_charger_stat;
 
 	int num_charger_regulators;
 	struct charger_regulator *charger_regulators;
 
-	char *psy_fuel_gauge;
+	const char *psy_fuel_gauge;
 
 	int temp_alert_min;
 	int temp_alert_max;
@@ -222,8 +222,8 @@ struct charger_desc {
 	int (*temperature_out_of_range)(struct charger_manager *);
 	bool measure_battery_temp;
 
-	u64 charging_max_duration_ms;
-	u64 discharging_max_duration_ms;
+	u32 charging_max_duration_ms;
+	u32 discharging_max_duration_ms;
 };
 
 #define PSY_NAME_MAX	30
-- 
1.7.9.5

Re: [PATCH 1/3] charger-manager : Replace kzalloc to devm_kzalloc and remove uneccessary code.

[Anton Vorontsov]

On Fri, Oct 25, 2013 at 11:47:31AM +0900, Jonghwa Lee wrote:
> Use devm function for dynamic memory allocation.
> 
> Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
> ---

1/3 applied, thanks!

Anton

Re: [PATCH 2/3] charger-manager : Add default battery temperature checking funtion.

[Anton Vorontsov]

On Fri, Oct 25, 2013 at 11:47:32AM +0900, Jonghwa Lee wrote:
> During the charger manager driver's probing time, it can't succeed
> if there's no pre-defined .temperature_out_of_range callback function.
> But if fuel gauge supports battery temperature measurement, we
> can use it directly. That's what cm_default_get_temp() function does.
> 
> With flag measure_batter_temp ON, we normally use cm_default_get_temp()
> for .temperature_out_of_range callback funtion.
> The TEMP_AMBIENT property is only used for pre-defined one.
> 
> Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
> ---
...
> +static int cm_default_get_temp(struct charger_manager *cm)
> +{
> +	struct charger_desc *desc = cm->desc;
> +	union power_supply_propval val;
> +	static int temp_alert_min = 0;

No. Never.

> +	static int temp_alert_max = 0;
> +	static int temp_alert_diff = 0;
> +	static int last_temp_status = 0;
> +	int ret;
> +
> +	if (!temp_alert_min && !temp_alert_max) {
> +		/* Initialize minimum temperature for alert */
> +		ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
> +					POWER_SUPPLY_PROP_TEMP_ALERT_MIN,
> +					&val);
> +		temp_alert_min = ret ? desc->temp_alert_min :
> +					min(desc->temp_alert_min, val.intval);

Is it a minimum temperature of a bettery below which we should alert?
Should it be max() then?

> +		if (!temp_alert_min)
> +			temp_alert_min = CM_DEFAULT_TEMP_ALERT_MIN;

The whole function is ugly as hell.  Please tidy it up, split it into two,
three or whatever amount needed to make it readable and comprehendable.
Use temporary variables for things like
cm->fuel_gauge->get_property(cm->fuel_gauge, ....).

> +
> +		/* Initialize maximum temperature for alert */
> +		ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
> +					POWER_SUPPLY_PROP_TEMP_ALERT_MAX,
> +					&val);
> +		temp_alert_max = ret ? desc->temp_alert_max :
> +					min(desc->temp_alert_max, val.intval);

Thanks,

Anton

Re: [PATCH 1/3] charger-manager : Replace kzalloc to devm_kzalloc and remove uneccessary code.

[jonghwa3.lee]

On 2013년 10월 26일 08:30, Anton Vorontsov wrote:

> On Fri, Oct 25, 2013 at 11:47:31AM +0900, Jonghwa Lee wrote:
>> Use devm function for dynamic memory allocation.
>>
>> Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
>> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
>> ---
> 
> 1/3 applied, thanks!
> 
> Anton


Thanks,
Jonghwa

> 

Re: [PATCH 2/3] charger-manager : Add default battery temperature checking funtion.

[jonghwa3.lee]

On 2013년 10월 26일 08:44, Anton Vorontsov wrote:

> On Fri, Oct 25, 2013 at 11:47:32AM +0900, Jonghwa Lee wrote:
>> During the charger manager driver's probing time, it can't succeed
>> if there's no pre-defined .temperature_out_of_range callback function.
>> But if fuel gauge supports battery temperature measurement, we
>> can use it directly. That's what cm_default_get_temp() function does.
>>
>> With flag measure_batter_temp ON, we normally use cm_default_get_temp()
>> for .temperature_out_of_range callback funtion.
>> The TEMP_AMBIENT property is only used for pre-defined one.
>>
>> Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
>> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
>> ---
> ...
>> +static int cm_default_get_temp(struct charger_manager *cm)
>> +{
>> +	struct charger_desc *desc = cm->desc;
>> +	union power_supply_propval val;
>> +	static int temp_alert_min = 0;
> 
> No. Never.


Can I assume that you worried about initialization of a variable which would be
used to hold minimum value to zero not INT_MAX or something big?
Anyway, come to think of it, it looks ugly for me either. I'll fix it.

> 
>> +	static int temp_alert_max = 0;
>> +	static int temp_alert_diff = 0;
>> +	static int last_temp_status = 0;
>> +	int ret;
>> +
>> +	if (!temp_alert_min && !temp_alert_max) {
>> +		/* Initialize minimum temperature for alert */
>> +		ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
>> +					POWER_SUPPLY_PROP_TEMP_ALERT_MIN,
>> +					&val);
>> +		temp_alert_min = ret ? desc->temp_alert_min :
>> +					min(desc->temp_alert_min, val.intval);
> 
> Is it a minimum temperature of a bettery below which we should alert?
> Should it be max() then?


Yes, you're right. I'll fix it.

> 
>> +		if (!temp_alert_min)
>> +			temp_alert_min = CM_DEFAULT_TEMP_ALERT_MIN;
> 
> The whole function is ugly as hell.  Please tidy it up, split it into two,
> three or whatever amount needed to make it readable and comprehendable.
> Use temporary variables for things like
> cm->fuel_gauge->get_property(cm->fuel_gauge, ....).
> 


Okay, I'll re-post it.

Thanks for comments
Jonghwa.

>> +
>> +		/* Initialize maximum temperature for alert */
>> +		ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
>> +					POWER_SUPPLY_PROP_TEMP_ALERT_MAX,
>> +					&val);
>> +		temp_alert_max = ret ? desc->temp_alert_max :
>> +					min(desc->temp_alert_max, val.intval);
> 
> Thanks,
> 
> Anton
> 

Re: [PATCH 2/3] charger-manager : Add default battery temperature checking funtion.

[Anton Vorontsov]

On Mon, Oct 28, 2013 at 11:26:18AM +0900, jonghwa3.lee@samsung.com wrote:
> >> During the charger manager driver's probing time, it can't succeed
> >> if there's no pre-defined .temperature_out_of_range callback function.
> >> But if fuel gauge supports battery temperature measurement, we
> >> can use it directly. That's what cm_default_get_temp() function does.
> >>
> >> With flag measure_batter_temp ON, we normally use cm_default_get_temp()
> >> for .temperature_out_of_range callback funtion.
> >> The TEMP_AMBIENT property is only used for pre-defined one.
> >>
> >> Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
> >> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
> >> ---
> > ...
> >> +static int cm_default_get_temp(struct charger_manager *cm)
> >> +{
> >> +	struct charger_desc *desc = cm->desc;
> >> +	union power_supply_propval val;
> >> +	static int temp_alert_min = 0;
> >
> > No. Never.
> 
> Can I assume that you worried about initialization of a variable which would be
> used to hold minimum value to zero not INT_MAX or something big?

Nah, it's just any static variable inside a function is a big no-no. :)
With a very rare exceptions (mostly boot/arch code, which is not the case
here).

Basically, if you have to use static variables inside a function, it means
that you just broke device-driver separation.

Thanks,

Anton