[PATCH 0/4] charger-manager: Support Devicetree and use IIO susbystem

[PATCH 0/4] charger-manager: Support Devicetree and use IIO susbystem

[Chanwoo Choi]

This patchset support DT(Device Tree) to get platform data for charger desc
structure from dts file and use IIO(Industrial I/O) subsystem to read ADC
value of battery temperature instead of using legacy method.

Chanwoo Choi (4):
  charger-manager: Support DT to get platform data for charger_desc
  charger-manager: Use IIO subsystem to read battery temperature instead of legacy method
  charger-manager: Add device tree binding for charger-manager
  charger-manager: Remove build warning fo unused variable

 .../devicetree/bindings/power/charger-manager.txt  | 106 ++++++
 drivers/power/Kconfig                              |   1 +
 drivers/power/charger-manager.c                    | 379 ++++++++++++++++++++-
 include/linux/power/charger-manager.h              |  25 +-
 4 files changed, 493 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/charger-manager.txt

-- 
1.8.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/4] charger-manager: Support Devicetree and use IIO susbystem

[Chanwoo Choi]

This patchset support DT(Device Tree) to get platform data for charger desc
structure from dts file and use IIO(Industrial I/O) subsystem to read ADC
value of battery temperature instead of using legacy method.

Chanwoo Choi (4):
  charger-manager: Support DT to get platform data for charger_desc
  charger-manager: Use IIO subsystem to read battery temperature instead of legacy method
  charger-manager: Add device tree binding for charger-manager
  charger-manager: Remove build warning fo unused variable

 .../devicetree/bindings/power/charger-manager.txt  | 106 ++++++
 drivers/power/Kconfig                              |   1 +
 drivers/power/charger-manager.c                    | 379 ++++++++++++++++++++-
 include/linux/power/charger-manager.h              |  25 +-
 4 files changed, 493 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/charger-manager.txt

-- 
1.8.0

[PATCH 1/4] charger-manager: Support DT to get platform data for charger_desc

[Chanwoo Choi]

This patch support DT(Device Tree) to get platform data for charger_desc
structure which includes necessary properties to control charger/fuel-gague
power-supply device.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
---
 drivers/power/charger-manager.c       | 290 +++++++++++++++++++++++++++++++++-
 include/linux/power/charger-manager.h |  12 +-
 2 files changed, 290 insertions(+), 12 deletions(-)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index e30e847..cc720f9 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -25,6 +25,7 @@
 #include <linux/power/charger-manager.h>
 #include <linux/regulator/consumer.h>
 #include <linux/sysfs.h>
+#include <linux/of.h>
 
 static const char * const default_event_names[] = {
 	[CM_EVENT_UNKNOWN] = "Unknown",
@@ -518,7 +519,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 %dms\n",
 				 desc->charging_max_duration_ms);
 			uevent_notify(cm, "Discharging");
 			try_charger_enable(cm, false);
@@ -529,7 +530,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 %dms\n",
 				 desc->discharging_max_duration_ms);
 			uevent_notify(cm, "Recharging");
 			try_charger_enable(cm, true);
@@ -1136,7 +1137,8 @@ static void charger_extcon_work(struct work_struct *work)
 					cable->min_uA, cable->max_uA);
 		if (ret < 0) {
 			pr_err("Cannot set current limit of %s (%s)\n",
-			       cable->charger->regulator_name, cable->name);
+			       cable->charger->regulator_name,
+			       cable->cable_name);
 			return;
 		}
 
@@ -1206,10 +1208,10 @@ static int charger_extcon_init(struct charger_manager *cm,
 	INIT_WORK(&cable->wq, charger_extcon_work);
 	cable->nb.notifier_call = charger_extcon_notifier;
 	ret = extcon_register_interest(&cable->extcon_dev,
-			cable->extcon_name, cable->name, &cable->nb);
+			cable->extcon_name, cable->cable_name, &cable->nb);
 	if (ret < 0) {
 		pr_info("Cannot register extcon_dev for %s(cable: %s)\n",
-			cable->extcon_name, cable->name);
+			cable->extcon_name, cable->cable_name);
 		ret = -EINVAL;
 	}
 
@@ -1438,14 +1440,280 @@ err:
 	return ret;
 }
 
+#ifdef CONFIG_OF
+static int charger_manager_dt_parse_psy_charger(struct device *dev,
+					       struct charger_desc *desc)
+{
+	struct device_node *np, *psy_chargers_np, *charger_np;
+	int i, num_psy_chargers;
+
+	np = dev->of_node;
+
+	/* Get platform data of Charger Regulators from DT */
+	psy_chargers_np = of_find_node_by_name(np, "psy-chargers");
+	if (!psy_chargers_np) {
+		dev_err(dev, "cannot find psy-chargers sub-node\n");
+		return -EINVAL;
+	}
+
+	num_psy_chargers = of_get_child_count(psy_chargers_np);
+	if (!num_psy_chargers) {
+		dev_err(dev, "cannot get the child count of psy-chargers\n");
+		return -EINVAL;
+	}
+
+	desc->psy_charger_stat = devm_kzalloc(dev,
+				sizeof(*desc->psy_charger_stat) *
+				num_psy_chargers, GFP_KERNEL);
+	if (!desc->psy_charger_stat) {
+		dev_err(dev, "cannot allocate memory for psy-chargers\n");
+		return -EINVAL;
+	}
+
+	i = 0;
+	for_each_child_of_node(psy_chargers_np, charger_np) {
+		if (of_property_read_string(charger_np, "psy-charger-name",
+					&desc->psy_charger_stat[i])) {
+			dev_err(dev, "cannot get psy-charger name\n");
+			return -EINVAL;
+		}
+		dev_dbg(dev, "psy-charger.%d (%s)\n",
+				i, desc->psy_charger_stat[i]);
+		i++;
+	}
+
+	return 0;
+}
+
+static int charger_manager_dt_parse_regulator(struct device *dev,
+					       struct charger_desc *desc)
+{
+	struct device_node *np, *charger_regulators_np, *charger_cables_np;
+	struct device_node *regulator_np, *cable_np;
+	int i, j;
+	int num_charger_regulators;
+
+	np = dev->of_node;
+
+	/* Get platform data of Charger Regulators from DT */
+	charger_regulators_np = of_find_node_by_name(np, "charger-regulators");
+	if (!charger_regulators_np) {
+		dev_err(dev, "cannot find charger-regulators sub-node\n");
+		return -EINVAL;
+	}
+
+	num_charger_regulators = of_get_child_count(charger_regulators_np);
+	if (!num_charger_regulators) {
+		dev_err(dev, "cannot get child count of charger-regulators\n");
+		return -EINVAL;
+	}
+	desc->num_charger_regulators = num_charger_regulators;
+
+	desc->charger_regulators = devm_kzalloc(dev,
+				sizeof(*desc->charger_regulators) *
+				desc->num_charger_regulators, GFP_KERNEL);
+	if (!desc->charger_regulators) {
+		dev_err(dev, "cannot allocate memory for charger-regulators\n");
+		return -EINVAL;
+	}
+
+	i = 0;
+	for_each_child_of_node(charger_regulators_np, regulator_np) {
+		struct charger_regulator *charger
+				= &desc->charger_regulators[i];
+
+		if (of_property_read_string(regulator_np, "regulator-name",
+					    &charger->regulator_name)) {
+			dev_err(dev, "cannot get power supply name\n");
+			return -EINVAL;
+		}
+
+		dev_dbg(dev, "charger.%d (%s)\n", i, charger->regulator_name);
+
+		/* Get platform data of Charger Cables from DT */
+		charger_cables_np = of_find_node_by_name(regulator_np,
+							"charger-cables");
+		if (!charger_cables_np) {
+			dev_err(dev, "cannot find charger-cables sub-node\n");
+			i++;
+			continue;
+		}
+		charger->num_cables = of_get_child_count(charger_cables_np);
+		if (!charger->num_cables) {
+			dev_err(dev,
+				"cannot get child count of charger-cables\n");
+			return -EINVAL;
+		}
+
+		charger->cables = devm_kzalloc(dev, sizeof(*charger->cables) *
+					       charger->num_cables, GFP_KERNEL);
+		if (!charger->cables) {
+			dev_err(dev,
+				"cannot allocate memory for charger-cables\n");
+			return -EINVAL;
+		}
+
+		j = 0;
+		for_each_child_of_node(charger_cables_np, cable_np) {
+			struct charger_cable *cable =  &charger->cables[j];
+
+			of_property_read_string(cable_np, "cable-name",
+						&cable->cable_name);
+			of_property_read_string(cable_np, "extcon-name",
+						&cable->extcon_name);
+			of_property_read_u32(cable_np, "min-current-uA",
+					     &cable->min_uA);
+			of_property_read_u32(cable_np, "max-current-uA",
+					     &cable->max_uA);
+
+			if (!cable->cable_name || !cable->extcon_name ||
+				!cable->min_uA || !cable->max_uA) {
+				dev_err(dev,
+					"cannot get datas for charger-cable\n");
+				return -EINVAL;
+			}
+
+			dev_dbg(dev, "\tcable.%d(%s)\n", j, cable->cable_name);
+			dev_dbg(dev, "\tcable.%d(%s)\n", j, cable->extcon_name);
+			dev_dbg(dev, "\tcable.%d(%d uA)\n", j, cable->min_uA);
+			dev_dbg(dev, "\tcable.%d(%d uA)\n\n", j, cable->max_uA);
+
+			j++;
+		}
+		i++;
+	}
+
+	return 0;
+}
+
+static struct charger_desc *charger_manager_dt_parse(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct charger_desc *desc;
+
+	if (!np)
+		return NULL;
+
+	desc = devm_kzalloc(dev, sizeof(struct charger_desc), GFP_KERNEL);
+	if (!desc) {
+		dev_err(dev, "Failed to allocate memory\n");
+		return NULL;
+	}
+
+	if (of_property_read_string(np, "psy-name", &desc->psy_name)) {
+		dev_err(dev, "cannot get power supply name\n");
+		return NULL;
+	}
+
+	if (of_property_read_u32(np, "polling-mode", &desc->polling_mode)) {
+		dev_warn(dev, "cannot get tyep of polling mode\n");
+		desc->polling_mode = CM_POLL_EXTERNAL_POWER_ONLY;
+	}
+
+	if (of_property_read_u32(np, "polling-interval-ms",
+				&desc->polling_interval_ms)) {
+		dev_warn(dev, "cannot get tyep of polling interval time\n");
+		desc->polling_interval_ms = 30000;
+	}
+
+	if (of_property_read_u32(np, "fullbatt-vchkdrop-ms",
+				&desc->fullbatt_vchkdrop_ms)) {
+
+		dev_err(dev, "cannot get fullbatt-vchkdrop-ms\n");
+		return NULL;
+	}
+
+	if (of_property_read_u32(np, "fullbatt-vchkdrop-uV",
+				&desc->fullbatt_vchkdrop_uV)) {
+		dev_err(dev, "cannot get fullbatt-vchkdrop-ms\n");
+		return NULL;
+	}
+
+	if (of_property_read_u32(np, "fullbatt-uV",
+				&desc->fullbatt_uV)) {
+		dev_err(dev, "cannot get fullbatt-uV\n");
+		return NULL;
+	}
+
+	if (of_property_read_u32(np, "fullbatt-soc",
+				&desc->fullbatt_soc)) {
+		dev_warn(dev, "cannot get fullbatt-soc\n");
+		desc->fullbatt_soc = 100;
+	}
+
+	if (of_property_read_u32(np, "fullbatt-full-capacity",
+				&desc->fullbatt_full_capacity)) {
+		dev_warn(dev, "cannot get fullbatt-full-capacity\n");
+		desc->fullbatt_full_capacity = 0;
+	}
+
+	if (of_property_read_u32(np, "battery-present",
+				&desc->battery_present)) {
+		dev_warn(dev, "cannot get tyep of battery present\n");
+		desc->battery_present = CM_CHARGER_STAT;
+	}
+
+	if (charger_manager_dt_parse_psy_charger(dev, desc)) {
+		dev_err(dev, "cannot parse the power-supply charger\n");
+		return NULL;
+	}
+
+	if (charger_manager_dt_parse_regulator(dev, desc)) {
+		dev_err(dev, "cannot parse the charger-regulators/cables\n");
+		return NULL;
+	}
+
+	if (of_property_read_string(np, "psy-fuelgauge-name",
+				&desc->psy_fuel_gauge)) {
+		dev_err(dev, "cannot get fuelgauge name\n");
+		return NULL;
+	}
+
+	if (of_property_read_bool(np, "measure-battery-temp"))
+		desc->measure_battery_temp = true;
+
+	if (of_property_read_u32(np, "charging-max-duration-minute",
+				&desc->charging_max_duration_ms)) {
+		dev_err(dev, "cannot get charging-max-duration-minute\n");
+		return NULL;
+	}
+
+	if (of_property_read_u32(np, "discharging-max-duration-minute",
+				&desc->discharging_max_duration_ms)) {
+		dev_err(dev, "cannot get discharging-max-duration-minute\n");
+		return NULL;
+	}
+
+	/* Convert unit from minute to millisecond */
+	desc->charging_max_duration_ms *= (60 * 1000);
+	desc->discharging_max_duration_ms *= (60 * 1000);
+
+	return desc;
+}
+#else
+#define charger_manager_parse_dt	NULL
+#endif
+
 static int charger_manager_probe(struct platform_device *pdev)
 {
-	struct charger_desc *desc = dev_get_platdata(&pdev->dev);
+	struct charger_desc *desc;
 	struct charger_manager *cm;
 	int ret = 0, i = 0;
 	int j = 0;
 	union power_supply_propval val;
 
+	if (pdev->dev.of_node)
+		desc = charger_manager_dt_parse(&pdev->dev);
+	else if (pdev->dev.platform_data)
+		desc = dev_get_platdata(&pdev->dev);
+	else
+		desc = NULL;
+
+	if (!desc) {
+		dev_err(&pdev->dev, "Failed to get charger_desc data\n");
+		return -EINVAL;
+	}
+
 	if (g_desc && !rtc_dev && g_desc->rtc_name) {
 		rtc_dev = rtc_class_open(g_desc->rtc_name);
 		if (IS_ERR_OR_NULL(rtc_dev)) {
@@ -1834,11 +2102,21 @@ static const struct dev_pm_ops charger_manager_pm = {
 	.complete	= cm_suspend_complete,
 };
 
+#ifdef CONFIG_OF
+static struct of_device_id charger_manager_id_match[] = {
+	{ .compatible = "charger-manager", },
+	{},
+};
+#else
+#define charger_manager_id_match	NULL
+#endif
+
 static struct platform_driver charger_manager_driver = {
 	.driver = {
 		.name = "charger-manager",
 		.owner = THIS_MODULE,
 		.pm = &charger_manager_pm,
+		.of_match_table = charger_manager_id_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 0e86840..8696fb8 100644
--- a/include/linux/power/charger-manager.h
+++ b/include/linux/power/charger-manager.h
@@ -83,7 +83,7 @@ struct charger_global_desc {
  */
 struct charger_cable {
 	const char *extcon_name;
-	const char *name;
+	const char *cable_name;
 
 	/* The charger-manager use Exton framework*/
 	struct extcon_specific_cable_nb extcon_dev;
@@ -190,7 +190,7 @@ struct charger_regulator {
  *	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;
@@ -203,18 +203,18 @@ 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 (*temperature_out_of_range)(int *mC);
 	bool measure_battery_temp;
 
-	u64 charging_max_duration_ms;
-	u64 discharging_max_duration_ms;
+	unsigned int charging_max_duration_ms;
+	unsigned int discharging_max_duration_ms;
 };
 
 #define PSY_NAME_MAX	30
-- 
1.8.0

[PATCH 2/4] charger-manager: Use IIO subsystem to read battery temperature instead of legacy method

[Chanwoo Choi]

This patch support charger-manager use IIO(Industrial I/O) subsystem to read
current battery temperature instead of legacy methor about callback function.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
---
 drivers/power/Kconfig                 |  1 +
 drivers/power/charger-manager.c       | 88 +++++++++++++++++++++++++++++++++--
 include/linux/power/charger-manager.h | 13 ++++++
 3 files changed, 97 insertions(+), 5 deletions(-)

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index e6f92b4..6700191 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -309,6 +309,7 @@ config CHARGER_MANAGER
 	bool "Battery charger manager for multiple chargers"
 	depends on REGULATOR && RTC_CLASS
 	select EXTCON
+	select IIO
 	help
           Say Y to enable charger-manager support, which allows multiple
           chargers attached to a battery and multiple batteries attached to a
diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index cc720f9..02a395c 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -26,6 +26,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/sysfs.h>
 #include <linux/of.h>
+#include <linux/iio/consumer.h>
 
 static const char * const default_event_names[] = {
 	[CM_EVENT_UNKNOWN] = "Unknown",
@@ -542,6 +543,50 @@ static int check_charging_duration(struct charger_manager *cm)
 }
 
 /**
+ * read_battery_temperature - Read current battery temperature
+ * @cm: the Charger Manager representing the battery.
+ * @last_temp_mC: store current battery temperature
+ *
+ * Returns current state of temperature by using IIO or legacy method
+ * - CM_TEMP_NORMAL
+ * - CM_TEMP_OVERHEAT
+ * - CM_TEMP_COLD
+ */
+static int read_battery_temperature(struct charger_manager *cm,
+				    int *last_temp_mC)
+{
+	struct charger_desc *desc = cm->desc;
+	int temp;
+
+	if (desc->channel) {
+		temp = iio_read_channel_raw(desc->channel, last_temp_mC);
+
+		/*
+		 * The charger-manager use IIO subsystem to read ADC raw data
+		 * from IIO ADC device drvier. The each device driver has
+		 * own non-standard ADC table. If user of charger-manager
+		 * would like to get correct temperature value, have to convert
+		 * 'last_temp_mC' variable according to proper calculation
+		 * method and own ADC table.
+		 */
+
+		if (*last_temp_mC >= desc->iio_adc_overheat)
+			temp = CM_TEMP_NORMAL;	/* Overheat */
+		else if (*last_temp_mC <= desc->iio_adc_cold)
+			temp = CM_TEMP_COLD;	/* Cold */
+		else
+			temp = CM_TEMP_NORMAL;	/* Normal */
+
+	} else if (desc->temperature_out_of_range) {
+		temp = desc->temperature_out_of_range(last_temp_mC);
+	} else {
+		temp = INT_MAX;
+	}
+
+	return temp;
+}
+
+/**
  * _cm_monitor - Monitor the temperature and return true for exceptions.
  * @cm: the Charger Manager representing the battery.
  *
@@ -551,7 +596,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 = read_battery_temperature(cm, &cm->last_temp_mC);
 
 	dev_dbg(cm->dev, "monitoring (%2.2d.%3.3dC)\n",
 		cm->last_temp_mC / 1000, cm->last_temp_mC % 1000);
@@ -805,7 +850,7 @@ static int charger_get_property(struct power_supply *psy,
 	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);
+			read_battery_temperature(cm, &cm->last_temp_mC);
 		val->intval = cm->last_temp_mC / 100;
 		if (!desc->measure_battery_temp)
 			ret = -ENODEV;
@@ -813,7 +858,7 @@ static int charger_get_property(struct power_supply *psy,
 	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);
+			read_battery_temperature(cm, &cm->last_temp_mC);
 		val->intval = cm->last_temp_mC / 100;
 		if (desc->measure_battery_temp)
 			ret = -ENODEV;
@@ -1586,6 +1631,32 @@ static int charger_manager_dt_parse_regulator(struct device *dev,
 	return 0;
 }
 
+static int charger_manager_dt_parse_iio(struct device *dev,
+					      struct charger_desc *desc)
+{
+	struct device_node *np = dev->of_node;
+
+	if (of_property_read_u32(np, "iio-adc-overheat",
+				&desc->iio_adc_overheat)) {
+		dev_warn(dev, "cannot get standard value for hot temperature\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(np, "iio-adc-cold",
+				&desc->iio_adc_cold)) {
+		dev_warn(dev, "cannot get standard value for cold temperature\n");
+		return -EINVAL;
+	}
+
+	desc->channel = iio_channel_get(dev, NULL);
+	if (IS_ERR(desc->channel)) {
+		dev_err(dev, "cannot get iio channel\n");
+		return PTR_ERR(desc->channel);
+	}
+
+	return 0;
+}
+
 static struct charger_desc *charger_manager_dt_parse(struct device *dev)
 {
 	struct device_node *np = dev->of_node;
@@ -1688,6 +1759,11 @@ static struct charger_desc *charger_manager_dt_parse(struct device *dev)
 	desc->charging_max_duration_ms *= (60 * 1000);
 	desc->discharging_max_duration_ms *= (60 * 1000);
 
+	if (charger_manager_dt_parse_iio(dev, desc)) {
+		dev_err(dev, "cannot get iio device to read temperature\n");
+		return NULL;
+	}
+
 	return desc;
 }
 #else
@@ -1814,8 +1890,8 @@ static int charger_manager_probe(struct platform_device *pdev)
 		goto err_chg_stat;
 	}
 
-	if (!desc->temperature_out_of_range) {
-		dev_err(&pdev->dev, "there is no temperature_out_of_range\n");
+	if (!desc->channel && !desc->temperature_out_of_range) {
+		dev_err(&pdev->dev, "there is no temperature function\n");
 		ret = -EINVAL;
 		goto err_chg_stat;
 	}
@@ -1986,6 +2062,8 @@ static int charger_manager_remove(struct platform_device *pdev)
 
 	try_charger_enable(cm, false);
 
+	iio_channel_release(desc->channel);
+
 	kfree(cm->charger_psy.properties);
 	kfree(cm->charger_stat);
 	kfree(cm->desc);
diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
index 8696fb8..33dfc69 100644
--- a/include/linux/power/charger-manager.h
+++ b/include/linux/power/charger-manager.h
@@ -42,6 +42,12 @@ enum cm_event_types {
 	CM_EVENT_OTHERS,
 };
 
+enum cm_temperature_types {
+	CM_TEMP_COLD = -1,
+	CM_TEMP_NORMAL = 0,
+	CM_TEMP_OVERHEAT = 1,
+};
+
 /**
  * struct charger_global_desc
  * @rtc_name: the name of RTC used to wake up the system from suspend.
@@ -188,6 +194,9 @@ struct charger_regulator {
  *	Maximum possible duration for discharging with charger cable
  *	after full-batt. If discharging duration exceed 'discharging
  *	max_duration_ms', cm start charging.
+ * @channel: filled with a channel from iio
+ * @iio_adc_overheat: the value of the highest ADC for temperature
+ * @iio_adc_cold: the value of the lowest ADC for temperature
  */
 struct charger_desc {
 	const char *psy_name;
@@ -215,6 +224,10 @@ struct charger_desc {
 
 	unsigned int charging_max_duration_ms;
 	unsigned int discharging_max_duration_ms;
+
+	struct iio_channel *channel;
+	int iio_adc_overheat;
+	int iio_adc_cold;
 };
 
 #define PSY_NAME_MAX	30
-- 
1.8.0

Re: [PATCH 2/4] charger-manager: Use IIO subsystem to read battery temperature instead of legacy method

[Lars-Peter Clausen]

On 10/22/2013 02:51 PM, Chanwoo Choi wrote:
> This patch support charger-manager use IIO(Industrial I/O) subsystem to read
> current battery temperature instead of legacy methor about callback function.

How does this look in hardware? Do you have a general purpose ADC connected
to a temperature sensor that has a temperature dependent voltage output? And
at some point during the board design you measure the raw value of the ADC
both for the lower and the upper threshold temperatures?

> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
> ---
>  drivers/power/Kconfig                 |  1 +
>  drivers/power/charger-manager.c       | 88 +++++++++++++++++++++++++++++++++--
>  include/linux/power/charger-manager.h | 13 ++++++
>  3 files changed, 97 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index e6f92b4..6700191 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -309,6 +309,7 @@ config CHARGER_MANAGER
>  	bool "Battery charger manager for multiple chargers"
>  	depends on REGULATOR && RTC_CLASS
>  	select EXTCON
> +	select IIO

Are you sure you want to force select the IIO framework? It looks like we do
not stub out iio_channel_get() and friends yet if CONFIG_IIO is not
selected. But I think that will the better solution here.

>  	help
>            Say Y to enable charger-manager support, which allows multiple
>            chargers attached to a battery and multiple batteries attached to a
> diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
> index cc720f9..02a395c 100644
> --- a/drivers/power/charger-manager.c
> +++ b/drivers/power/charger-manager.c
> @@ -26,6 +26,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/sysfs.h>
>  #include <linux/of.h>
> +#include <linux/iio/consumer.h>
>  
>  static const char * const default_event_names[] = {
>  	[CM_EVENT_UNKNOWN] = "Unknown",
> @@ -542,6 +543,50 @@ static int check_charging_duration(struct charger_manager *cm)
>  }
>  
>  /**
> + * read_battery_temperature - Read current battery temperature
> + * @cm: the Charger Manager representing the battery.
> + * @last_temp_mC: store current battery temperature
> + *
> + * Returns current state of temperature by using IIO or legacy method
> + * - CM_TEMP_NORMAL
> + * - CM_TEMP_OVERHEAT
> + * - CM_TEMP_COLD
> + */
> +static int read_battery_temperature(struct charger_manager *cm,
> +				    int *last_temp_mC)
> +{
> +	struct charger_desc *desc = cm->desc;
> +	int temp;
> +
> +	if (desc->channel) {
> +		temp = iio_read_channel_raw(desc->channel, last_temp_mC);
> +
> +		/*
> +		 * The charger-manager use IIO subsystem to read ADC raw data
> +		 * from IIO ADC device drvier. The each device driver has
> +		 * own non-standard ADC table. If user of charger-manager
> +		 * would like to get correct temperature value, have to convert
> +		 * 'last_temp_mC' variable according to proper calculation
> +		 * method and own ADC table.
> +		 */
> +
> +		if (*last_temp_mC >= desc->iio_adc_overheat)
> +			temp = CM_TEMP_NORMAL;	/* Overheat */

temp = CM_TEMP_OVERHEAT ?

> +		else if (*last_temp_mC <= desc->iio_adc_cold)
> +			temp = CM_TEMP_COLD;	/* Cold */
> +		else
> +			temp = CM_TEMP_NORMAL;	/* Normal */
> +
> +	} else if (desc->temperature_out_of_range) {
> +		temp = desc->temperature_out_of_range(last_temp_mC);
> +	} else {
> +		temp = INT_MAX;
> +	}
> +
> +	return temp;
> +}
> +
> +/**
>   * _cm_monitor - Monitor the temperature and return true for exceptions.
>   * @cm: the Charger Manager representing the battery.
>   *
> @@ -551,7 +596,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 = read_battery_temperature(cm, &cm->last_temp_mC);
>  
>  	dev_dbg(cm->dev, "monitoring (%2.2d.%3.3dC)\n",
>  		cm->last_temp_mC / 1000, cm->last_temp_mC % 1000);
> @@ -805,7 +850,7 @@ static int charger_get_property(struct power_supply *psy,
>  	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);
> +			read_battery_temperature(cm, &cm->last_temp_mC);

So this will now return the raw ADC value to userspace on a property that is
supposed to indicate milli-degree Celsius. That doesn't seem to be right.

>  		val->intval = cm->last_temp_mC / 100;
>  		if (!desc->meaure_battery_temp)
>  			ret = -ENODEV;
> @@ -813,7 +858,7 @@ static int charger_get_property(struct power_supply *psy,
>  	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);
> +			read_battery_temperature(cm, &cm->last_temp_mC);
>  		val->intval = cm->last_temp_mC / 100;
>  		if (desc->measure_battery_temp)
>  			ret = -ENODEV;
> @@ -1586,6 +1631,32 @@ static int charger_manager_dt_parse_regulator(struct device *dev,
>  	return 0;
>  }
>  
> +static int charger_manager_dt_parse_iio(struct device *dev,
> +					      struct charger_desc *desc)
> +{
> +	struct device_node *np = dev->of_node;
> +
> +	if (of_property_read_u32(np, "iio-adc-overheat",
> +				&desc->iio_adc_overheat)) {
> +		dev_warn(dev, "cannot get standard value for hot temperature\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(np, "iio-adc-cold",
> +				&desc->iio_adc_cold)) {
> +		dev_warn(dev, "cannot get standard value for cold temperature\n");
> +		return -EINVAL;
> +	}

iio-adc-overheat and iio-adc-cold are definitely not good names for
devicetree properties. The term IIO is really Linux specific and should not
appear in the devicetree. 'temperature-lower-threshold' and
'temperature-upper-threshold' might be better names.

> +
> +	desc->channel = iio_channel_get(dev, NULL);

You need to free the channel on the error paths in your probe function.

> +	if (IS_ERR(desc->channel)) {
> +		dev_err(dev, "cannot get iio channel\n");
> +		return PTR_ERR(desc->channel);
> +	}
> +
> +	return 0;
> +}

Re: [PATCH 2/4] charger-manager: Use IIO subsystem to read battery temperature instead of legacy method

[Chanwoo Choi]

On 10/26/2013 08:20 PM, Lars-Peter Clausen wrote:
> On 10/22/2013 02:51 PM, Chanwoo Choi wrote:
>> This patch support charger-manager use IIO(Industrial I/O) subsystem to read
>> current battery temperature instead of legacy methor about callback function.
> 
> How does this look in hardware? Do you have a general purpose ADC connected
> to a temperature sensor that has a temperature dependent voltage output? And
> at some point during the board design you measure the raw value of the ADC
> both for the lower and the upper threshold temperatures?

As you comment, I have to convert ADC value with h/w constraint(voltage ouput, resistance).
Previously, I used exynos-adc driver(drivers/iio/adc/exynos_adc.c) to get ADC value
about temperature and then use ntc_thermistor(drivers/hwmon/ntc_thermistor.c)
for converting temperature. But, I didn't find same driver of ntc_thermistor
in drivers/iio for converting. So, this patchset only connected with iio drvier
to get ADC value. In next, I'm cosidering how to get the correct temperature from iio driver.

If you possible, could you give me advices about this issue?

Also, I will support POWER_SUPPLY_PROP_TEMP* property of power_supply class
to get temperature from fuel-gauge or charger device.


> 
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
>> ---
>>  drivers/power/Kconfig                 |  1 +
>>  drivers/power/charger-manager.c       | 88 +++++++++++++++++++++++++++++++++--
>>  include/linux/power/charger-manager.h | 13 ++++++
>>  3 files changed, 97 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
>> index e6f92b4..6700191 100644
>> --- a/drivers/power/Kconfig
>> +++ b/drivers/power/Kconfig
>> @@ -309,6 +309,7 @@ config CHARGER_MANAGER
>>  	bool "Battery charger manager for multiple chargers"
>>  	depends on REGULATOR && RTC_CLASS
>>  	select EXTCON
>> +	select IIO
> 
> Are you sure you want to force select the IIO framework? It looks like we do
> not stub out iio_channel_get() and friends yet if CONFIG_IIO is not
> selected. But I think that will the better solution here.
> 
>>  	help
>>            Say Y to enable charger-manager support, which allows multiple
>>            chargers attached to a battery and multiple batteries attached to a
>> diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
>> index cc720f9..02a395c 100644
>> --- a/drivers/power/charger-manager.c
>> +++ b/drivers/power/charger-manager.c
>> @@ -26,6 +26,7 @@
>>  #include <linux/regulator/consumer.h>
>>  #include <linux/sysfs.h>
>>  #include <linux/of.h>
>> +#include <linux/iio/consumer.h>
>>  
>>  static const char * const default_event_names[] = {
>>  	[CM_EVENT_UNKNOWN] = "Unknown",
>> @@ -542,6 +543,50 @@ static int check_charging_duration(struct charger_manager *cm)
>>  }
>>  
>>  /**
>> + * read_battery_temperature - Read current battery temperature
>> + * @cm: the Charger Manager representing the battery.
>> + * @last_temp_mC: store current battery temperature
>> + *
>> + * Returns current state of temperature by using IIO or legacy method
>> + * - CM_TEMP_NORMAL
>> + * - CM_TEMP_OVERHEAT
>> + * - CM_TEMP_COLD
>> + */
>> +static int read_battery_temperature(struct charger_manager *cm,
>> +				    int *last_temp_mC)
>> +{
>> +	struct charger_desc *desc = cm->desc;
>> +	int temp;
>> +
>> +	if (desc->channel) {
>> +		temp = iio_read_channel_raw(desc->channel, last_temp_mC);
>> +
>> +		/*
>> +		 * The charger-manager use IIO subsystem to read ADC raw data
>> +		 * from IIO ADC device drvier. The each device driver has
>> +		 * own non-standard ADC table. If user of charger-manager
>> +		 * would like to get correct temperature value, have to convert
>> +		 * 'last_temp_mC' variable according to proper calculation
>> +		 * method and own ADC table.
>> +		 */
>> +
>> +		if (*last_temp_mC >= desc->iio_adc_overheat)
>> +			temp = CM_TEMP_NORMAL;	/* Overheat */
> 
> temp = CM_TEMP_OVERHEAT ?

I found this problem after send patchset. I'll fix it.

> 
>> +		else if (*last_temp_mC <= desc->iio_adc_cold)
>> +			temp = CM_TEMP_COLD;	/* Cold */
>> +		else
>> +			temp = CM_TEMP_NORMAL;	/* Normal */
>> +
>> +	} else if (desc->temperature_out_of_range) {
>> +		temp = desc->temperature_out_of_range(last_temp_mC);
>> +	} else {
>> +		temp = INT_MAX;
>> +	}
>> +
>> +	return temp;
>> +}
>> +
>> +/**
>>   * _cm_monitor - Monitor the temperature and return true for exceptions.
>>   * @cm: the Charger Manager representing the battery.
>>   *
>> @@ -551,7 +596,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 = read_battery_temperature(cm, &cm->last_temp_mC);
>>  
>>  	dev_dbg(cm->dev, "monitoring (%2.2d.%3.3dC)\n",
>>  		cm->last_temp_mC / 1000, cm->last_temp_mC % 1000);
>> @@ -805,7 +850,7 @@ static int charger_get_property(struct power_supply *psy,
>>  	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);
>> +			read_battery_temperature(cm, &cm->last_temp_mC);
> 
> So this will now return the raw ADC value to userspace on a property that is
> supposed to indicate milli-degree Celsius. That doesn't seem to be right.

You're right. It isn't correct temperature as below my comment:
I have to fix this issue.

		/*
		 * The charger-manager use IIO subsystem to read ADC raw data
		 * from IIO ADC device drvier. The each device driver has
		 * own non-standard ADC table. If user of charger-manager
		 * would like to get correct temperature value, have to convert
		 * 'last_temp_mC' variable according to proper calculation
		 * method and own ADC table.
		 */

> 
>>  		val->intval = cm->last_temp_mC / 100;
>>  		if (!desc->meaure_battery_temp)
>>  			ret = -ENODEV;
>> @@ -813,7 +858,7 @@ static int charger_get_property(struct power_supply *psy,
>>  	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);
>> +			read_battery_temperature(cm, &cm->last_temp_mC);
>>  		val->intval = cm->last_temp_mC / 100;
>>  		if (desc->measure_battery_temp)
>>  			ret = -ENODEV;
>> @@ -1586,6 +1631,32 @@ static int charger_manager_dt_parse_regulator(struct device *dev,
>>  	return 0;
>>  }
>>  
>> +static int charger_manager_dt_parse_iio(struct device *dev,
>> +					      struct charger_desc *desc)
>> +{
>> +	struct device_node *np = dev->of_node;
>> +
>> +	if (of_property_read_u32(np, "iio-adc-overheat",
>> +				&desc->iio_adc_overheat)) {
>> +		dev_warn(dev, "cannot get standard value for hot temperature\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (of_property_read_u32(np, "iio-adc-cold",
>> +				&desc->iio_adc_cold)) {
>> +		dev_warn(dev, "cannot get standard value for cold temperature\n");
>> +		return -EINVAL;
>> +	}
> 
> iio-adc-overheat and iio-adc-cold are definitely not good names for
> devicetree properties. The term IIO is really Linux specific and should not
> appear in the devicetree. 'temperature-lower-threshold' and
> 'temperature-upper-threshold' might be better names.

OK. I'll fix it in accordance with your comment.

> 
>> +
>> +	desc->channel = iio_channel_get(dev, NULL);
> 
> You need to free the channel on the error paths in your probe function.

OK. I'll fix it.
> 
>> +	if (IS_ERR(desc->channel)) {
>> +		dev_err(dev, "cannot get iio channel\n");
>> +		return PTR_ERR(desc->channel);
>> +	}
>> +
>> +	return 0;
>> +}
> 

Thanks for your comment.

Best Regards,
Chanwoo Choi

Re: [PATCH 2/4] charger-manager: Use IIO subsystem to read battery temperature instead of legacy method

[Chanwoo Choi]

On 10/26/2013 08:20 PM, Lars-Peter Clausen wrote:
> On 10/22/2013 02:51 PM, Chanwoo Choi wrote:
>> This patch support charger-manager use IIO(Industrial I/O) subsystem to read
>> current battery temperature instead of legacy methor about callback function.
> 
> How does this look in hardware? Do you have a general purpose ADC connected
> to a temperature sensor that has a temperature dependent voltage output? And
> at some point during the board design you measure the raw value of the ADC
> both for the lower and the upper threshold temperatures?

As you comment, I have to convert ADC value with h/w constraint(voltage ouput, resistance).
Previously, I used exynos-adc driver(drivers/iio/adc/exynos_adc.c) to get ADC value
about temperature and then use ntc_thermistor(drivers/hwmon/ntc_thermistor.c)
for converting temperature. But, I didn't find same driver of ntc_thermistor
in drivers/iio for converting. So, this patchset only connected with iio drvier
to get ADC value. In next, I'm cosidering how to get the correct temperature from iio driver.

If you possible, could you give me advices about this issue?

Also, I will support POWER_SUPPLY_PROP_TEMP* property of power_supply class
to get temperature from fuel-gauge or charger device.


> 
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Myungjoo Ham <myungjoo.ham-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>>  drivers/power/Kconfig                 |  1 +
>>  drivers/power/charger-manager.c       | 88 +++++++++++++++++++++++++++++++++--
>>  include/linux/power/charger-manager.h | 13 ++++++
>>  3 files changed, 97 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
>> index e6f92b4..6700191 100644
>> --- a/drivers/power/Kconfig
>> +++ b/drivers/power/Kconfig
>> @@ -309,6 +309,7 @@ config CHARGER_MANAGER
>>  	bool "Battery charger manager for multiple chargers"
>>  	depends on REGULATOR && RTC_CLASS
>>  	select EXTCON
>> +	select IIO
> 
> Are you sure you want to force select the IIO framework? It looks like we do
> not stub out iio_channel_get() and friends yet if CONFIG_IIO is not
> selected. But I think that will the better solution here.
> 
>>  	help
>>            Say Y to enable charger-manager support, which allows multiple
>>            chargers attached to a battery and multiple batteries attached to a
>> diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
>> index cc720f9..02a395c 100644
>> --- a/drivers/power/charger-manager.c
>> +++ b/drivers/power/charger-manager.c
>> @@ -26,6 +26,7 @@
>>  #include <linux/regulator/consumer.h>
>>  #include <linux/sysfs.h>
>>  #include <linux/of.h>
>> +#include <linux/iio/consumer.h>
>>  
>>  static const char * const default_event_names[] = {
>>  	[CM_EVENT_UNKNOWN] = "Unknown",
>> @@ -542,6 +543,50 @@ static int check_charging_duration(struct charger_manager *cm)
>>  }
>>  
>>  /**
>> + * read_battery_temperature - Read current battery temperature
>> + * @cm: the Charger Manager representing the battery.
>> + * @last_temp_mC: store current battery temperature
>> + *
>> + * Returns current state of temperature by using IIO or legacy method
>> + * - CM_TEMP_NORMAL
>> + * - CM_TEMP_OVERHEAT
>> + * - CM_TEMP_COLD
>> + */
>> +static int read_battery_temperature(struct charger_manager *cm,
>> +				    int *last_temp_mC)
>> +{
>> +	struct charger_desc *desc = cm->desc;
>> +	int temp;
>> +
>> +	if (desc->channel) {
>> +		temp = iio_read_channel_raw(desc->channel, last_temp_mC);
>> +
>> +		/*
>> +		 * The charger-manager use IIO subsystem to read ADC raw data
>> +		 * from IIO ADC device drvier. The each device driver has
>> +		 * own non-standard ADC table. If user of charger-manager
>> +		 * would like to get correct temperature value, have to convert
>> +		 * 'last_temp_mC' variable according to proper calculation
>> +		 * method and own ADC table.
>> +		 */
>> +
>> +		if (*last_temp_mC >= desc->iio_adc_overheat)
>> +			temp = CM_TEMP_NORMAL;	/* Overheat */
> 
> temp = CM_TEMP_OVERHEAT ?

I found this problem after send patchset. I'll fix it.

> 
>> +		else if (*last_temp_mC <= desc->iio_adc_cold)
>> +			temp = CM_TEMP_COLD;	/* Cold */
>> +		else
>> +			temp = CM_TEMP_NORMAL;	/* Normal */
>> +
>> +	} else if (desc->temperature_out_of_range) {
>> +		temp = desc->temperature_out_of_range(last_temp_mC);
>> +	} else {
>> +		temp = INT_MAX;
>> +	}
>> +
>> +	return temp;
>> +}
>> +
>> +/**
>>   * _cm_monitor - Monitor the temperature and return true for exceptions.
>>   * @cm: the Charger Manager representing the battery.
>>   *
>> @@ -551,7 +596,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 = read_battery_temperature(cm, &cm->last_temp_mC);
>>  
>>  	dev_dbg(cm->dev, "monitoring (%2.2d.%3.3dC)\n",
>>  		cm->last_temp_mC / 1000, cm->last_temp_mC % 1000);
>> @@ -805,7 +850,7 @@ static int charger_get_property(struct power_supply *psy,
>>  	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);
>> +			read_battery_temperature(cm, &cm->last_temp_mC);
> 
> So this will now return the raw ADC value to userspace on a property that is
> supposed to indicate milli-degree Celsius. That doesn't seem to be right.

You're right. It isn't correct temperature as below my comment:
I have to fix this issue.

		/*
		 * The charger-manager use IIO subsystem to read ADC raw data
		 * from IIO ADC device drvier. The each device driver has
		 * own non-standard ADC table. If user of charger-manager
		 * would like to get correct temperature value, have to convert
		 * 'last_temp_mC' variable according to proper calculation
		 * method and own ADC table.
		 */

> 
>>  		val->intval = cm->last_temp_mC / 100;
>>  		if (!desc->meaure_battery_temp)
>>  			ret = -ENODEV;
>> @@ -813,7 +858,7 @@ static int charger_get_property(struct power_supply *psy,
>>  	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);
>> +			read_battery_temperature(cm, &cm->last_temp_mC);
>>  		val->intval = cm->last_temp_mC / 100;
>>  		if (desc->measure_battery_temp)
>>  			ret = -ENODEV;
>> @@ -1586,6 +1631,32 @@ static int charger_manager_dt_parse_regulator(struct device *dev,
>>  	return 0;
>>  }
>>  
>> +static int charger_manager_dt_parse_iio(struct device *dev,
>> +					      struct charger_desc *desc)
>> +{
>> +	struct device_node *np = dev->of_node;
>> +
>> +	if (of_property_read_u32(np, "iio-adc-overheat",
>> +				&desc->iio_adc_overheat)) {
>> +		dev_warn(dev, "cannot get standard value for hot temperature\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (of_property_read_u32(np, "iio-adc-cold",
>> +				&desc->iio_adc_cold)) {
>> +		dev_warn(dev, "cannot get standard value for cold temperature\n");
>> +		return -EINVAL;
>> +	}
> 
> iio-adc-overheat and iio-adc-cold are definitely not good names for
> devicetree properties. The term IIO is really Linux specific and should not
> appear in the devicetree. 'temperature-lower-threshold' and
> 'temperature-upper-threshold' might be better names.

OK. I'll fix it in accordance with your comment.

> 
>> +
>> +	desc->channel = iio_channel_get(dev, NULL);
> 
> You need to free the channel on the error paths in your probe function.

OK. I'll fix it.
> 
>> +	if (IS_ERR(desc->channel)) {
>> +		dev_err(dev, "cannot get iio channel\n");
>> +		return PTR_ERR(desc->channel);
>> +	}
>> +
>> +	return 0;
>> +}
> 

Thanks for your comment.

Best Regards,
Chanwoo Choi

Re: [PATCH 2/4] charger-manager: Use IIO subsystem to read battery temperature instead of legacy method

[Pavel Machek]

Hi!

> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index e6f92b4..6700191 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -309,6 +309,7 @@ config CHARGER_MANAGER
>  	bool "Battery charger manager for multiple chargers"
>  	depends on REGULATOR && RTC_CLASS
>  	select EXTCON
> +	select IIO
>  	help
>            Say Y to enable charger-manager support, which allows multiple
>            chargers attached to a battery and multiple batteries attached to a

Umm. Are there charger-manager users that don't have temperature sensor on IIO?

> +	if (desc->channel) {
> +		temp = iio_read_channel_raw(desc->channel, last_temp_mC);
> +
> +		/*
> +		 * The charger-manager use IIO subsystem to read ADC raw data
> +		 * from IIO ADC device drvier. The each device driver has
> +		 * own non-standard ADC table. If user of charger-manager
> +		 * would like to get correct temperature value, have to convert
> +		 * 'last_temp_mC' variable according to proper calculation
> +		 * method and own ADC table.
> +		 */
> +
> +		if (*last_temp_mC >= desc->iio_adc_overheat)
> +			temp = CM_TEMP_NORMAL;	/* Overheat */
> +		else if (*last_temp_mC <= desc->iio_adc_cold)
> +			temp = CM_TEMP_COLD;	/* Cold */
> +		else
> +			temp = CM_TEMP_NORMAL;	/* Normal */

Something is definitely wrong here.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 2/4] charger-manager: Use IIO subsystem to read battery temperature instead of legacy method

[Chanwoo Choi]

Hi Pavel,

On 10/27/2013 08:17 PM, Pavel Machek wrote:
> Hi!
> 
>> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
>> index e6f92b4..6700191 100644
>> --- a/drivers/power/Kconfig
>> +++ b/drivers/power/Kconfig
>> @@ -309,6 +309,7 @@ config CHARGER_MANAGER
>>  	bool "Battery charger manager for multiple chargers"
>>  	depends on REGULATOR && RTC_CLASS
>>  	select EXTCON
>> +	select IIO
>>  	help
>>            Say Y to enable charger-manager support, which allows multiple
>>            chargers attached to a battery and multiple batteries attached to a
> 
> Umm. Are there charger-manager users that don't have temperature sensor on IIO?

I'll remove tightly coupled dependency between IIO and Charger-manager.

> 
>> +	if (desc->channel) {
>> +		temp = iio_read_channel_raw(desc->channel, last_temp_mC);
>> +
>> +		/*
>> +		 * The charger-manager use IIO subsystem to read ADC raw data
>> +		 * from IIO ADC device drvier. The each device driver has
>> +		 * own non-standard ADC table. If user of charger-manager
>> +		 * would like to get correct temperature value, have to convert
>> +		 * 'last_temp_mC' variable according to proper calculation
>> +		 * method and own ADC table.
>> +		 */
>> +
>> +		if (*last_temp_mC >= desc->iio_adc_overheat)
>> +			temp = CM_TEMP_NORMAL;	/* Overheat */
>> +		else if (*last_temp_mC <= desc->iio_adc_cold)
>> +			temp = CM_TEMP_COLD;	/* Cold */
>> +		else
>> +			temp = CM_TEMP_NORMAL;	/* Normal */
> 
> Something is definitely wrong here.

I'll fix it as below:
temp = CM_TEMP_NORMAL;	/* Overheat */
-> temp = CM_TEMP_OVERHEAT;	/* Overheat */

Also, as you mentioned, this patch haven't included the converting sequence
from ADC value to correct temperature because currently iio driver haven't
support this feature. So, I'm going to discuss this issue on following mailing thread:
- https://lkml.org/lkml/2013/10/26/42

If I find proper method to get temperature on IIO driver, I'll modify it.

Thanks for your comment.

Best Regards,
Chanwoo Choi

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] charger-manager: Use IIO subsystem to read battery temperature instead of legacy method

[Chanwoo Choi]

Hi Pavel,

On 10/27/2013 08:17 PM, Pavel Machek wrote:
> Hi!
> 
>> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
>> index e6f92b4..6700191 100644
>> --- a/drivers/power/Kconfig
>> +++ b/drivers/power/Kconfig
>> @@ -309,6 +309,7 @@ config CHARGER_MANAGER
>>  	bool "Battery charger manager for multiple chargers"
>>  	depends on REGULATOR && RTC_CLASS
>>  	select EXTCON
>> +	select IIO
>>  	help
>>            Say Y to enable charger-manager support, which allows multiple
>>            chargers attached to a battery and multiple batteries attached to a
> 
> Umm. Are there charger-manager users that don't have temperature sensor on IIO?

I'll remove tightly coupled dependency between IIO and Charger-manager.

> 
>> +	if (desc->channel) {
>> +		temp = iio_read_channel_raw(desc->channel, last_temp_mC);
>> +
>> +		/*
>> +		 * The charger-manager use IIO subsystem to read ADC raw data
>> +		 * from IIO ADC device drvier. The each device driver has
>> +		 * own non-standard ADC table. If user of charger-manager
>> +		 * would like to get correct temperature value, have to convert
>> +		 * 'last_temp_mC' variable according to proper calculation
>> +		 * method and own ADC table.
>> +		 */
>> +
>> +		if (*last_temp_mC >= desc->iio_adc_overheat)
>> +			temp = CM_TEMP_NORMAL;	/* Overheat */
>> +		else if (*last_temp_mC <= desc->iio_adc_cold)
>> +			temp = CM_TEMP_COLD;	/* Cold */
>> +		else
>> +			temp = CM_TEMP_NORMAL;	/* Normal */
> 
> Something is definitely wrong here.

I'll fix it as below:
temp = CM_TEMP_NORMAL;	/* Overheat */
-> temp = CM_TEMP_OVERHEAT;	/* Overheat */

Also, as you mentioned, this patch haven't included the converting sequence
from ADC value to correct temperature because currently iio driver haven't
support this feature. So, I'm going to discuss this issue on following mailing thread:
- https://lkml.org/lkml/2013/10/26/42

If I find proper method to get temperature on IIO driver, I'll modify it.

Thanks for your comment.

Best Regards,
Chanwoo Choi

[PATCH 3/4] charger-manager: Add device tree binding for charger-manager

[Chanwoo Choi]

This patch add binding file for charger-manager that this framework enables to
control and multiple chargers and to monitor charging event.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
---
 .../devicetree/bindings/power/charger-manager.txt  | 106 +++++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/charger-manager.txt

diff --git a/Documentation/devicetree/bindings/power/charger-manager.txt b/Documentation/devicetree/bindings/power/charger-manager.txt
new file mode 100644
index 0000000..b22c5526
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/charger-manager.txt
@@ -0,0 +1,106 @@
+Charger-manager framework
+
+THe devicetree bindings are for the charger-manager to control charging feature.
+
+This framework enables to control and multiple chargers and to monitor charging
+event in the context of suspend-to-RAM with an interface combining the chargers.
+
+Required Properties:
+- compatible:		Must be "charger-manager".
+- psy-name:		The name of power-supply class for charger-manager.
+- polling-mode:		Determine which polling mode will be used.
+- polling-invertal-ms:	Interval in millisecond at which charger-manager will
+			monitor battery health.
+- fullbatt-vchkdrop-ms
+- fullbatt-vchkdrop-uV:	Check voltage drop afer the battery is fully charged.
+			If it has dropped more than fullbatt_vchkdrop_uV after
+			fullbatt_vchkdrop_ms, charger-manager will restart
+			charging.
+- fullbatt-uV:		The standard voltage in microvolt for checking
+			fully charged. If VBATT >= fullbatt_uV, it is assumed to
+			be full.
+- fullbatt-soc:		The standard SOC(State Of Charger) for checking
+			fully charged. If state of Charge >= fullbatt_soc, it is
+			assumed to be full.
+- fullbatt-full-capacity: The standard capacity for checking fully charged.
+			If full capacity of battery >= fullbatt_full_capacity,
+			it is assumed to be full.
+- battery-present:	Specify where information for existance of battery
+			can be obtained.
+- measure-battery-temp:	Whether to measure battery or not
+
+- charging-max-duration-minute:		Maximum possible duration for charging.
+			If whole charging duration exceed 'charging-max-duration
+			-ms', charger-manager stop charging.
+- discharging-max-duration-minute:	Maximum possible duration for
+			discharging with charger cable after full-batt. If
+			discharging duration exceed 'discharging-max-duration-
+			ms', charger-manager start charging.
+- psy-fuelgauge-name:	The name of power-supply for fuel gauge
+- io-channels:		The iio channel data for ADC
+- iio-adc-overheat:	The value of the highest ADC for temperature
+- iio-adc-cold:		The value of the lowest ADC for temperature
+- psy-chargers:		Arrary of power-supply name for chargers.
+- charger-regulators:	Array of charger regulators. It include charger cable
+			data which need cable-name/extcon-name/min-current-uA
+			max-current-uA. When cable is attached/detached,
+			charger-manager change current limit of regulator
+			according to min-current-uA/max-current-uA.
+
+Examples: adding charger-desc data in dts file
+
+charger-manager@ {
+	compatible = "charger-manager";
+
+	psy-name = "battery";
+
+	polling-mode = <2>;		/* Polling external power */
+	polling-interval-ms = <30000>;	/* Polling period */
+
+	fullbatt-vchkdrop-ms = <30000>;
+	fullbatt-vchkdrop-uV = <150000>;
+	fullbatt-uV = <420000>;
+	fullbatt-soc = <100>;
+	fullbatt-full-capacity = <0>;
+
+	battery-present = <3>;		/* CM_CHARGER_STAT */
+
+	measure-battery-temp;
+
+	charging-max-duration-minute = <360>;
+	discharging-max-duration-minute = <90>;
+
+	psy-fuelgauge-name = "max17040";
+
+	io-channels = <&adc 2>;
+	iio-adc-overheat = <2500>;
+	iio-adc-cold = <0>;
+
+	psy-chargers {
+		psy-charger-max14577 {
+			psy-charger-name = "max14577-charger";
+		};
+	};
+
+	charger-regulators {
+		charger-vinchg1 {
+			regulator-name = "vinchg1";
+
+			charger-cables {
+				cable-USB {
+					cable-name = "USB";
+					extcon-name = "max14577-muic";
+					min-current-uA = <475000>;
+					max-current-uA = <500000>;
+				};
+
+				cable-TA {
+					cable-name = "TA";
+					extcon-name = "max14577-muic";
+					min-current-uA = <650000>;
+					max-current-uA = <675000>;
+				};
+			};
+		};
+	};
+};
-- 
1.8.0

Re: [PATCH 3/4] charger-manager: Add device tree binding for charger-manager

[Grant Likely]

On Tue, 22 Oct 2013 21:51:56 +0900, Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> This patch add binding file for charger-manager that this framework enables to
> control and multiple chargers and to monitor charging event.
> 
> Signed-off-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Myungjoo Ham <myungjoo.ham-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  .../devicetree/bindings/power/charger-manager.txt  | 106 +++++++++++++++++++++
>  1 file changed, 106 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/charger-manager.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/charger-manager.txt b/Documentation/devicetree/bindings/power/charger-manager.txt
> new file mode 100644
> index 0000000..b22c5526
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/charger-manager.txt
> @@ -0,0 +1,106 @@
> +Charger-manager framework
> +
> +THe devicetree bindings are for the charger-manager to control charging feature.
> +
> +This framework enables to control and multiple chargers and to monitor charging
> +event in the context of suspend-to-RAM with an interface combining the chargers.
> +
> +Required Properties:
> +- compatible:		Must be "charger-manager".
> +- psy-name:		The name of power-supply class for charger-manager.
> +- polling-mode:		Determine which polling mode will be used.
> +- polling-invertal-ms:	Interval in millisecond at which charger-manager will
> +			monitor battery health.
> +- fullbatt-vchkdrop-ms
> +- fullbatt-vchkdrop-uV:	Check voltage drop afer the battery is fully charged.
> +			If it has dropped more than fullbatt_vchkdrop_uV after
> +			fullbatt_vchkdrop_ms, charger-manager will restart
> +			charging.
> +- fullbatt-uV:		The standard voltage in microvolt for checking
> +			fully charged. If VBATT >= fullbatt_uV, it is assumed to
> +			be full.
> +- fullbatt-soc:		The standard SOC(State Of Charger) for checking
> +			fully charged. If state of Charge >= fullbatt_soc, it is
> +			assumed to be full.
> +- fullbatt-full-capacity: The standard capacity for checking fully charged.
> +			If full capacity of battery >= fullbatt_full_capacity,
> +			it is assumed to be full.
> +- battery-present:	Specify where information for existance of battery
> +			can be obtained.
> +- measure-battery-temp:	Whether to measure battery or not
> +
> +- charging-max-duration-minute:		Maximum possible duration for charging.
> +			If whole charging duration exceed 'charging-max-duration
> +			-ms', charger-manager stop charging.
> +- discharging-max-duration-minute:	Maximum possible duration for
> +			discharging with charger cable after full-batt. If
> +			discharging duration exceed 'discharging-max-duration-
> +			ms', charger-manager start charging.
> +- psy-fuelgauge-name:	The name of power-supply for fuel gauge
> +- io-channels:		The iio channel data for ADC
> +- iio-adc-overheat:	The value of the highest ADC for temperature
> +- iio-adc-cold:		The value of the lowest ADC for temperature
> +- psy-chargers:		Arrary of power-supply name for chargers.
> +- charger-regulators:	Array of charger regulators. It include charger cable
> +			data which need cable-name/extcon-name/min-current-uA
> +			max-current-uA. When cable is attached/detached,
> +			charger-manager change current limit of regulator
> +			according to min-current-uA/max-current-uA.

The documentation for the above properties needs to specify what the
values actually mean. For example, "polling-mode" is documented as
"Determine which polling mode will be used". Huh? What is the difference
between a value of 0 or 1? What values are valid? What do they mean?

As it stands I suspect that the binding is a direct translation from the
existing pdata structure. That probably isn't really what you want. Some
of the properties above do make sense and are documented correctly (ie.
fullbatt-*), but others don't make sense and probably shouldn't be part
of the generic binding (ie. io-channels sounds like something device
specific).

I'll defer to the regulators people on what does and does not make
sense.

g.

> +
> +Examples: adding charger-desc data in dts file
> +
> +charger-manager@ {
> +	compatible = "charger-manager";
> +
> +	psy-name = "battery";
> +
> +	polling-mode = <2>;		/* Polling external power */
> +	polling-interval-ms = <30000>;	/* Polling period */
> +
> +	fullbatt-vchkdrop-ms = <30000>;
> +	fullbatt-vchkdrop-uV = <150000>;
> +	fullbatt-uV = <420000>;
> +	fullbatt-soc = <100>;
> +	fullbatt-full-capacity = <0>;
> +
> +	battery-present = <3>;		/* CM_CHARGER_STAT */
> +
> +	measure-battery-temp;
> +
> +	charging-max-duration-minute = <360>;
> +	discharging-max-duration-minute = <90>;
> +
> +	psy-fuelgauge-name = "max17040";
> +
> +	io-channels = <&adc 2>;
> +	iio-adc-overheat = <2500>;
> +	iio-adc-cold = <0>;
> +
> +	psy-chargers {
> +		psy-charger-max14577 {
> +			psy-charger-name = "max14577-charger";
> +		};
> +	};
> +
> +	charger-regulators {
> +		charger-vinchg1 {
> +			regulator-name = "vinchg1";
> +
> +			charger-cables {
> +				cable-USB {
> +					cable-name = "USB";
> +					extcon-name = "max14577-muic";
> +					min-current-uA = <475000>;
> +					max-current-uA = <500000>;
> +				};
> +
> +				cable-TA {
> +					cable-name = "TA";
> +					extcon-name = "max14577-muic";
> +					min-current-uA = <650000>;
> +					max-current-uA = <675000>;
> +				};
> +			};
> +		};
> +	};
> +};
> -- 
> 1.8.0
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] charger-manager: Add device tree binding for charger-manager

[Grant Likely]

On Tue, 22 Oct 2013 21:51:56 +0900, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> This patch add binding file for charger-manager that this framework enables to
> control and multiple chargers and to monitor charging event.
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
> ---
>  .../devicetree/bindings/power/charger-manager.txt  | 106 +++++++++++++++++++++
>  1 file changed, 106 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/charger-manager.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/charger-manager.txt b/Documentation/devicetree/bindings/power/charger-manager.txt
> new file mode 100644
> index 0000000..b22c5526
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/charger-manager.txt
> @@ -0,0 +1,106 @@
> +Charger-manager framework
> +
> +THe devicetree bindings are for the charger-manager to control charging feature.
> +
> +This framework enables to control and multiple chargers and to monitor charging
> +event in the context of suspend-to-RAM with an interface combining the chargers.
> +
> +Required Properties:
> +- compatible:		Must be "charger-manager".
> +- psy-name:		The name of power-supply class for charger-manager.
> +- polling-mode:		Determine which polling mode will be used.
> +- polling-invertal-ms:	Interval in millisecond at which charger-manager will
> +			monitor battery health.
> +- fullbatt-vchkdrop-ms
> +- fullbatt-vchkdrop-uV:	Check voltage drop afer the battery is fully charged.
> +			If it has dropped more than fullbatt_vchkdrop_uV after
> +			fullbatt_vchkdrop_ms, charger-manager will restart
> +			charging.
> +- fullbatt-uV:		The standard voltage in microvolt for checking
> +			fully charged. If VBATT >= fullbatt_uV, it is assumed to
> +			be full.
> +- fullbatt-soc:		The standard SOC(State Of Charger) for checking
> +			fully charged. If state of Charge >= fullbatt_soc, it is
> +			assumed to be full.
> +- fullbatt-full-capacity: The standard capacity for checking fully charged.
> +			If full capacity of battery >= fullbatt_full_capacity,
> +			it is assumed to be full.
> +- battery-present:	Specify where information for existance of battery
> +			can be obtained.
> +- measure-battery-temp:	Whether to measure battery or not
> +
> +- charging-max-duration-minute:		Maximum possible duration for charging.
> +			If whole charging duration exceed 'charging-max-duration
> +			-ms', charger-manager stop charging.
> +- discharging-max-duration-minute:	Maximum possible duration for
> +			discharging with charger cable after full-batt. If
> +			discharging duration exceed 'discharging-max-duration-
> +			ms', charger-manager start charging.
> +- psy-fuelgauge-name:	The name of power-supply for fuel gauge
> +- io-channels:		The iio channel data for ADC
> +- iio-adc-overheat:	The value of the highest ADC for temperature
> +- iio-adc-cold:		The value of the lowest ADC for temperature
> +- psy-chargers:		Arrary of power-supply name for chargers.
> +- charger-regulators:	Array of charger regulators. It include charger cable
> +			data which need cable-name/extcon-name/min-current-uA
> +			max-current-uA. When cable is attached/detached,
> +			charger-manager change current limit of regulator
> +			according to min-current-uA/max-current-uA.

The documentation for the above properties needs to specify what the
values actually mean. For example, "polling-mode" is documented as
"Determine which polling mode will be used". Huh? What is the difference
between a value of 0 or 1? What values are valid? What do they mean?

As it stands I suspect that the binding is a direct translation from the
existing pdata structure. That probably isn't really what you want. Some
of the properties above do make sense and are documented correctly (ie.
fullbatt-*), but others don't make sense and probably shouldn't be part
of the generic binding (ie. io-channels sounds like something device
specific).

I'll defer to the regulators people on what does and does not make
sense.

g.

> +
> +Examples: adding charger-desc data in dts file
> +
> +charger-manager@ {
> +	compatible = "charger-manager";
> +
> +	psy-name = "battery";
> +
> +	polling-mode = <2>;		/* Polling external power */
> +	polling-interval-ms = <30000>;	/* Polling period */
> +
> +	fullbatt-vchkdrop-ms = <30000>;
> +	fullbatt-vchkdrop-uV = <150000>;
> +	fullbatt-uV = <420000>;
> +	fullbatt-soc = <100>;
> +	fullbatt-full-capacity = <0>;
> +
> +	battery-present = <3>;		/* CM_CHARGER_STAT */
> +
> +	measure-battery-temp;
> +
> +	charging-max-duration-minute = <360>;
> +	discharging-max-duration-minute = <90>;
> +
> +	psy-fuelgauge-name = "max17040";
> +
> +	io-channels = <&adc 2>;
> +	iio-adc-overheat = <2500>;
> +	iio-adc-cold = <0>;
> +
> +	psy-chargers {
> +		psy-charger-max14577 {
> +			psy-charger-name = "max14577-charger";
> +		};
> +	};
> +
> +	charger-regulators {
> +		charger-vinchg1 {
> +			regulator-name = "vinchg1";
> +
> +			charger-cables {
> +				cable-USB {
> +					cable-name = "USB";
> +					extcon-name = "max14577-muic";
> +					min-current-uA = <475000>;
> +					max-current-uA = <500000>;
> +				};
> +
> +				cable-TA {
> +					cable-name = "TA";
> +					extcon-name = "max14577-muic";
> +					min-current-uA = <650000>;
> +					max-current-uA = <675000>;
> +				};
> +			};
> +		};
> +	};
> +};
> -- 
> 1.8.0
> 

Re: [PATCH 3/4] charger-manager: Add device tree binding for charger-manager

[Chanwoo Choi]

Hi Grant,

On 10/26/2013 05:03 AM, Grant Likely wrote:
> On Tue, 22 Oct 2013 21:51:56 +0900, Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>> This patch add binding file for charger-manager that this framework enables to
>> control and multiple chargers and to monitor charging event.
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Myungjoo Ham <myungjoo.ham-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>>  .../devicetree/bindings/power/charger-manager.txt  | 106 +++++++++++++++++++++
>>  1 file changed, 106 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/power/charger-manager.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/charger-manager.txt b/Documentation/devicetree/bindings/power/charger-manager.txt
>> new file mode 100644
>> index 0000000..b22c5526
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/charger-manager.txt
>> @@ -0,0 +1,106 @@
>> +Charger-manager framework
>> +
>> +THe devicetree bindings are for the charger-manager to control charging feature.
>> +
>> +This framework enables to control and multiple chargers and to monitor charging
>> +event in the context of suspend-to-RAM with an interface combining the chargers.
>> +
>> +Required Properties:
>> +- compatible:		Must be "charger-manager".
>> +- psy-name:		The name of power-supply class for charger-manager.
>> +- polling-mode:		Determine which polling mode will be used.
>> +- polling-invertal-ms:	Interval in millisecond at which charger-manager will
>> +			monitor battery health.
>> +- fullbatt-vchkdrop-ms
>> +- fullbatt-vchkdrop-uV:	Check voltage drop afer the battery is fully charged.
>> +			If it has dropped more than fullbatt_vchkdrop_uV after
>> +			fullbatt_vchkdrop_ms, charger-manager will restart
>> +			charging.
>> +- fullbatt-uV:		The standard voltage in microvolt for checking
>> +			fully charged. If VBATT >= fullbatt_uV, it is assumed to
>> +			be full.
>> +- fullbatt-soc:		The standard SOC(State Of Charger) for checking
>> +			fully charged. If state of Charge >= fullbatt_soc, it is
>> +			assumed to be full.
>> +- fullbatt-full-capacity: The standard capacity for checking fully charged.
>> +			If full capacity of battery >= fullbatt_full_capacity,
>> +			it is assumed to be full.
>> +- battery-present:	Specify where information for existance of battery
>> +			can be obtained.
>> +- measure-battery-temp:	Whether to measure battery or not
>> +
>> +- charging-max-duration-minute:		Maximum possible duration for charging.
>> +			If whole charging duration exceed 'charging-max-duration
>> +			-ms', charger-manager stop charging.
>> +- discharging-max-duration-minute:	Maximum possible duration for
>> +			discharging with charger cable after full-batt. If
>> +			discharging duration exceed 'discharging-max-duration-
>> +			ms', charger-manager start charging.
>> +- psy-fuelgauge-name:	The name of power-supply for fuel gauge
>> +- io-channels:		The iio channel data for ADC
>> +- iio-adc-overheat:	The value of the highest ADC for temperature
>> +- iio-adc-cold:		The value of the lowest ADC for temperature
>> +- psy-chargers:		Arrary of power-supply name for chargers.
>> +- charger-regulators:	Array of charger regulators. It include charger cable
>> +			data which need cable-name/extcon-name/min-current-uA
>> +			max-current-uA. When cable is attached/detached,
>> +			charger-manager change current limit of regulator
>> +			according to min-current-uA/max-current-uA.
> 
> The documentation for the above properties needs to specify what the
> values actually mean. For example, "polling-mode" is documented as
> "Determine which polling mode will be used". Huh? What is the difference
> between a value of 0 or 1? What values are valid? What do they mean?
> 
> As it stands I suspect that the binding is a direct translation from the
> existing pdata structure. That probably isn't really what you want. Some
> of the properties above do make sense and are documented correctly (ie.
> fullbatt-*), but others don't make sense and probably shouldn't be part
> of the generic binding (ie. io-channels sounds like something device
> specific).
> 
> I'll defer to the regulators people on what does and does not make
> sense.
> 

OK, I add detailed and easy description for improving the understanding
in accordance with your comment and will resend this patchset.

Thanks.

Best Regards,
Chanwoo Choi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] charger-manager: Add device tree binding for charger-manager

[Chanwoo Choi]

Hi Grant,

On 10/26/2013 05:03 AM, Grant Likely wrote:
> On Tue, 22 Oct 2013 21:51:56 +0900, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> This patch add binding file for charger-manager that this framework enables to
>> control and multiple chargers and to monitor charging event.
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
>> ---
>>  .../devicetree/bindings/power/charger-manager.txt  | 106 +++++++++++++++++++++
>>  1 file changed, 106 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/power/charger-manager.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/charger-manager.txt b/Documentation/devicetree/bindings/power/charger-manager.txt
>> new file mode 100644
>> index 0000000..b22c5526
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/charger-manager.txt
>> @@ -0,0 +1,106 @@
>> +Charger-manager framework
>> +
>> +THe devicetree bindings are for the charger-manager to control charging feature.
>> +
>> +This framework enables to control and multiple chargers and to monitor charging
>> +event in the context of suspend-to-RAM with an interface combining the chargers.
>> +
>> +Required Properties:
>> +- compatible:		Must be "charger-manager".
>> +- psy-name:		The name of power-supply class for charger-manager.
>> +- polling-mode:		Determine which polling mode will be used.
>> +- polling-invertal-ms:	Interval in millisecond at which charger-manager will
>> +			monitor battery health.
>> +- fullbatt-vchkdrop-ms
>> +- fullbatt-vchkdrop-uV:	Check voltage drop afer the battery is fully charged.
>> +			If it has dropped more than fullbatt_vchkdrop_uV after
>> +			fullbatt_vchkdrop_ms, charger-manager will restart
>> +			charging.
>> +- fullbatt-uV:		The standard voltage in microvolt for checking
>> +			fully charged. If VBATT >= fullbatt_uV, it is assumed to
>> +			be full.
>> +- fullbatt-soc:		The standard SOC(State Of Charger) for checking
>> +			fully charged. If state of Charge >= fullbatt_soc, it is
>> +			assumed to be full.
>> +- fullbatt-full-capacity: The standard capacity for checking fully charged.
>> +			If full capacity of battery >= fullbatt_full_capacity,
>> +			it is assumed to be full.
>> +- battery-present:	Specify where information for existance of battery
>> +			can be obtained.
>> +- measure-battery-temp:	Whether to measure battery or not
>> +
>> +- charging-max-duration-minute:		Maximum possible duration for charging.
>> +			If whole charging duration exceed 'charging-max-duration
>> +			-ms', charger-manager stop charging.
>> +- discharging-max-duration-minute:	Maximum possible duration for
>> +			discharging with charger cable after full-batt. If
>> +			discharging duration exceed 'discharging-max-duration-
>> +			ms', charger-manager start charging.
>> +- psy-fuelgauge-name:	The name of power-supply for fuel gauge
>> +- io-channels:		The iio channel data for ADC
>> +- iio-adc-overheat:	The value of the highest ADC for temperature
>> +- iio-adc-cold:		The value of the lowest ADC for temperature
>> +- psy-chargers:		Arrary of power-supply name for chargers.
>> +- charger-regulators:	Array of charger regulators. It include charger cable
>> +			data which need cable-name/extcon-name/min-current-uA
>> +			max-current-uA. When cable is attached/detached,
>> +			charger-manager change current limit of regulator
>> +			according to min-current-uA/max-current-uA.
> 
> The documentation for the above properties needs to specify what the
> values actually mean. For example, "polling-mode" is documented as
> "Determine which polling mode will be used". Huh? What is the difference
> between a value of 0 or 1? What values are valid? What do they mean?
> 
> As it stands I suspect that the binding is a direct translation from the
> existing pdata structure. That probably isn't really what you want. Some
> of the properties above do make sense and are documented correctly (ie.
> fullbatt-*), but others don't make sense and probably shouldn't be part
> of the generic binding (ie. io-channels sounds like something device
> specific).
> 
> I'll defer to the regulators people on what does and does not make
> sense.
> 

OK, I add detailed and easy description for improving the understanding
in accordance with your comment and will resend this patchset.

Thanks.

Best Regards,
Chanwoo Choi

[PATCH 4/4] charger-manager: Remove build warning fo unused variable

[Chanwoo Choi]

drivers/power/charger-manager.c: In function ‘_cm_monitor’:
drivers/power/charger-manager.c:598:23: warning: unused variable ‘desc’ [-Wunused-variable]

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
---
 drivers/power/charger-manager.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 02a395c..02b4d76 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -595,7 +595,6 @@ static int read_battery_temperature(struct charger_manager *cm,
  */
 static bool _cm_monitor(struct charger_manager *cm)
 {
-	struct charger_desc *desc = cm->desc;
 	int temp = read_battery_temperature(cm, &cm->last_temp_mC);
 
 	dev_dbg(cm->dev, "monitoring (%2.2d.%3.3dC)\n",
-- 
1.8.0