mfd: Implement devicetree support for AB8500 Btemp

mfd: Implement devicetree support for AB8500 Btemp

[Rajanikanth HV]

This patch adds device tree support for
battery temperature monitor driver

Signed-off-by: Rajanikanth H.V <rajanikanth.hv@stericsson.com>
---
 .../bindings/power_supply/ab8500/btemp.txt         |   52 ++++++
 arch/arm/boot/dts/dbx5x0.dtsi                      |    8 +
 drivers/mfd/ab8500-core.c                          |    1 +
 drivers/power/Kconfig                              |    6 -
 drivers/power/ab8500_bmdata.h                      |    4 +-
 drivers/power/ab8500_btemp.c                       |  181 ++++++++++++++++----
 6 files changed, 213 insertions(+), 39 deletions(-)
 create mode 100644
Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt

diff --git a/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
new file mode 100644
index 0000000..b214efc
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
@@ -0,0 +1,52 @@
+=== AB8500 Battery Temperature Monitor Driver ===
+
+The properties below describes the node for battery
+temperature monitor driver.
+
+Required Properties:
+- compatible = "stericsson,ab8500-btemp"
+
+supplied-to:
+	This is a logical binding w.r.t power supply event change
+	across energy-management-module drivers where in the
+	runtime battery properties are shared along with uevent
+	notification.
+	ref: di->btemp_psy.external_power_changed =
+		ab8500_btemp_external_power_changed;
+		ab8500_btemp.c
+
+	Need for this property:
+		btemp, fg and charger updates power-supply properties
+		based on the events listed above.
+		Event handler invokes power supply change notifier
+		which in-turn invokes registered power supply class call-back
+		based on the 'supplied_to' string.
+		ref:
+		power_supply_changed_work(..) ./drivers/power/power_supply_core.c
+
+	example:
+	ab8500-btemp {
+		/* Other enery management module */
+		supplied-to = "ab8500_chargalg", "ab8500_fg";
+		num_supplicants = <2>;
+	};
+
+thermister-interface:
+	'btemp' and 'batctrl' are the pins interfaced for battery temperature
+	measurement, btemp is used when NTC(negative temperature coefficient)
+	resister is interfaced external to battery and batctrl is used when
+	NTC resister is internal to battery.
+
+
+li-ion-9100-battery:
+	use this to add support for the 9100 Li-ION battery,
+	this adjust the bkup battery charger parameters
+	Note: this property is used for tablet version of snowball board.
+
+	example:
+	ab8500-btemp {
+		thermister-internal-to-battery = <1>;
+		li_ion_9100_battery = <0>;
+	};
+Note:
+interrupts are defined and registered in the driver
diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index d69c087..643e7fd 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -360,6 +360,14 @@
 					li_ion_9100  = <0>;
 				};

+				ab8500-btemp {
+					compatible = "stericsson,ab8500-btemp";
+					supplied_to = "ab8500_chargalg", "ab8500_fg";
+					num_supplicants = <2>;
+					thermister_on_batctrl = <1>;
+					li_ion_9100  = <0>;
+				};
+
 				ab8500-usb {
 					compatible = "stericsson,ab8500-usb";
 					interrupts = < 90 0x4
diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
index c413cfa..7ffba9b 100644
--- a/drivers/mfd/ab8500-core.c
+++ b/drivers/mfd/ab8500-core.c
@@ -1047,6 +1047,7 @@ static struct mfd_cell __devinitdata ab8500_bm_devs[] = {
 	},
 	{
 		.name = "ab8500-btemp",
+		.of_compatible = "stericsson,ab8500-btemp",
 		.num_resources = ARRAY_SIZE(ab8500_btemp_resources),
 		.resources = ab8500_btemp_resources,
 	},
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index c1892f3..1434871 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -304,12 +304,6 @@ config AB8500_BM
 	help
 	  Say Y to include support for AB8500 battery management.

-config AB8500_BATTERY_THERM_ON_BATCTRL
-	bool "Thermistor connected on BATCTRL ADC"
-	depends on AB8500_BM
-	help
-	  Say Y to enable battery temperature measurements using
-	  thermistor connected on BATCTRL ADC.
 endif # POWER_SUPPLY

 source "drivers/power/avs/Kconfig"
diff --git a/drivers/power/ab8500_bmdata.h b/drivers/power/ab8500_bmdata.h
index 748334a..0bd0a45 100644
--- a/drivers/power/ab8500_bmdata.h
+++ b/drivers/power/ab8500_bmdata.h
@@ -22,7 +22,7 @@ static struct abx500_res_to_temp temp_tbl_A_thermister[] = {
 	{65, 12500},
 };
 static struct abx500_res_to_temp temp_tbl_B_thermister[] = {
-	{-5, 165418},
+	{-5, 200000},
 	{ 0, 159024},
 	{ 5, 151921},
 	{10, 144300},
@@ -229,7 +229,7 @@ static struct abx500_battery_type bat_type_thermister[] = {
 },
 {
 	.name = POWER_SUPPLY_TECHNOLOGY_LIPO,
-	.resis_high = 165418,
+	.resis_high = 200000,
 	.resis_low = 82869,
 	.battery_resistance = 300,
 	.charge_full_design = 900,
diff --git a/drivers/power/ab8500_btemp.c b/drivers/power/ab8500_btemp.c
index bba3cca..ac08f1a 100644
--- a/drivers/power/ab8500_btemp.c
+++ b/drivers/power/ab8500_btemp.c
@@ -16,6 +16,7 @@
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/completion.h>
@@ -25,6 +26,7 @@
 #include <linux/mfd/abx500/ab8500-bm.h>
 #include <linux/mfd/abx500/ab8500-gpadc.h>
 #include <linux/jiffies.h>
+#include "ab8500_bmdata.h"

 #define VTVOUT_V			1800

@@ -960,42 +962,153 @@ static int __devexit ab8500_btemp_remove(struct
platform_device *pdev)
 	return 0;
 }

-static int __devinit ab8500_btemp_probe(struct platform_device *pdev)
+static int __devinit
+btemp_of_probe(struct device *dev,
+		struct device_node *np,
+		struct abx500_bm_plat_data *bm_pdata)
 {
-	int irq, i, ret = 0;
-	u8 val;
-	struct abx500_bm_plat_data *plat_data = pdev->dev.platform_data;
-	struct ab8500_btemp *di;
-
-	if (!plat_data) {
-		dev_err(&pdev->dev, "No platform data\n");
-		return -EINVAL;
+	u8	val;
+	u32	pval;
+	int	i;
+	int	ext_thermister, lion_battery, ret = 0;
+	const char *bm_dev_name;
+	struct	abx500_btemp_platform_data *btemp = bm_pdata->btemp;
+	struct	abx500_bm_data		   *bat;
+	struct	abx500_battery_type	   *btype;
+
+	ret = of_property_read_u32(np, "num_supplicants", &pval);
+	if (ret) {
+		dev_err(dev, "missing property num_supplicants\n");
+		ret = -EINVAL;
+		goto inval_pval;
 	}
-
-	di = kzalloc(sizeof(*di), GFP_KERNEL);
-	if (!di)
-		return -ENOMEM;
-
-	/* get parent data */
-	di->dev = &pdev->dev;
-	di->parent = dev_get_drvdata(pdev->dev.parent);
-	di->gpadc = ab8500_gpadc_get("ab8500-gpadc.0");
-
-	/* get btemp specific platform data */
-	di->pdata = plat_data->btemp;
-	if (!di->pdata) {
-		dev_err(di->dev, "no btemp platform data supplied\n");
+	btemp->num_supplicants = pval;
+	btemp->supplied_to =
+		devm_kzalloc(dev, btemp->num_supplicants *
+			sizeof(const char *), GFP_KERNEL);
+	if (btemp->supplied_to == NULL) {
+		dev_err(dev, "%s no mem for supplied_to\n", __func__);
+		ret = -ENOMEM;
+		goto inval_pval;
+	}
+	for (val = 0; val < btemp->num_supplicants; ++val)
+		if (of_property_read_string_index
+			(np, "supplied_to", val, &bm_dev_name) == 0)
+			*(btemp->supplied_to + val) = (char *)bm_dev_name;
+		else {
+			dev_err(dev, "insufficient number of supplied_to data found\n");
+			ret = -EINVAL;
+			goto free_dev_mem;
+		}
+	ret = of_property_read_u32(np, "thermister_on_batctrl", &pval);
+	if (ret) {
+		dev_err(dev, "missing property thermister_on_batctrl\n");
 		ret = -EINVAL;
-		goto free_device_info;
+		goto free_dev_mem;
+	}
+	bm_pdata->battery = &ab8500_bm_data;
+	bat = bm_pdata->battery;
+	ext_thermister = 0;
+	if (pval == 0) {
+		bat->n_btypes = 4;
+		bat->bat_type = bat_type_ext_thermister;
+		bat->adc_therm = ABx500_ADC_THERM_BATTEMP;
+		ext_thermister = 1;
+	}
+	ret = of_property_read_u32(np, "li_ion_9100", &pval);
+	if (ret) {
+		dev_err(dev, "missing property li_ion_9100\n");
+		ret = -EINVAL;
+		goto free_dev_mem;
+	}
+	lion_battery = 0;
+	if (pval == 1) {
+		bat->no_maintenance = true;
+		bat->chg_unknown_bat = true;
+		bat->bat_type[BATTERY_UNKNOWN].charge_full_design = 2600;
+		bat->bat_type[BATTERY_UNKNOWN].termination_vol = 4150;
+		bat->bat_type[BATTERY_UNKNOWN].recharge_vol = 4130;
+		bat->bat_type[BATTERY_UNKNOWN].normal_cur_lvl = 520;
+		bat->bat_type[BATTERY_UNKNOWN].normal_vol_lvl = 4200;
+		lion_battery = 1;
+	}
+	/* select the battery resolution table */
+	for (i = 0; i < bat->n_btypes; ++i) {
+		btype = (bat->bat_type + i);
+		if (ext_thermister) {
+			btype->batres_tbl =
+				temp_to_batres_tbl_ext_thermister;
+		} else if (lion_battery) {
+			btype->batres_tbl =
+				temp_to_batres_tbl_9100;
+		} else {
+			btype->batres_tbl =
+				temp_to_batres_tbl_thermister;
+		}
+	}
+	return ret;
+free_dev_mem:
+	devm_kfree(dev, btemp->supplied_to);
+inval_pval:
+	return ret;
+}
+
+static int __devinit ab8500_btemp_probe(struct platform_device *pdev)
+{
+	u8	val;
+	int	i;
+	int	irq, ret = 0;
+	struct	abx500_bm_plat_data *pdata = pdev->dev.platform_data;
+	struct	device_node	*np = pdev->dev.of_node;
+	struct	ab8500_btemp	*di;
+
+	di = devm_kzalloc(&pdev->dev, sizeof(*di), GFP_KERNEL);
+	if (!di) {
+		dev_err(&pdev->dev, "%s no mem for ab8500_btemp\n", __func__);
+		ret = -ENOMEM;
 	}

-	/* get battery specific platform data */
-	di->bat = plat_data->battery;
-	if (!di->bat) {
-		dev_err(di->dev, "no battery platform data supplied\n");
+	if (np) {
+		if (!pdata) {
+			pdata =
+			devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+			if (!pdata) {
+				dev_err(&pdev->dev,
+					"%s no mem for pdata\n", __func__);
+				ret = -ENOMEM;
+				goto err_no_mem;
+			}
+			pdata->btemp = devm_kzalloc(&pdev->dev,
+					sizeof(*pdata->btemp), GFP_KERNEL);
+			if (!pdata->btemp) {
+				devm_kfree(&pdev->dev, pdata);
+				dev_err(&pdev->dev,
+					"%s no mem for pdata->btemp\n",
+					__func__);
+				ret = -ENOMEM;
+				goto free_device_info;
+			}
+		}
+		/* get battery specific platform data */
+		ret = btemp_of_probe(&pdev->dev, np, pdata);
+		if (ret) {
+			devm_kfree(&pdev->dev, pdata->btemp);
+			devm_kfree(&pdev->dev, pdata);
+			goto free_device_info;
+		}
+	}
+	if (!pdata) {
+		dev_err(&pdev->dev,
+			"%s no btemp platform data found\n", __func__);
 		ret = -EINVAL;
 		goto free_device_info;
 	}
+	di->pdata = pdata->btemp;
+	di->bat = pdata->battery;
+	/* get parent data */
+	di->dev = &pdev->dev;
+	di->parent = dev_get_drvdata(pdev->dev.parent);
+	di->gpadc = ab8500_gpadc_get("ab8500-gpadc.0");

 	/* BTEMP supply */
 	di->btemp_psy.name = "ab8500_btemp";
@@ -1008,7 +1121,6 @@ static int __devinit ab8500_btemp_probe(struct
platform_device *pdev)
 	di->btemp_psy.external_power_changed =
 		ab8500_btemp_external_power_changed;

-
 	/* Create a work queue for the btemp */
 	di->btemp_wq =
 		create_singlethread_workqueue("ab8500_btemp_wq");
@@ -1065,7 +1177,7 @@ static int __devinit ab8500_btemp_probe(struct
platform_device *pdev)
 			IRQF_SHARED | IRQF_NO_SUSPEND,
 			ab8500_btemp_irq[i].name, di);

-		if (ret) {
+		if (ret < 0) {
 			dev_err(di->dev, "failed to request %s IRQ %d: %d\n"
 				, ab8500_btemp_irq[i].name, irq, ret);
 			goto free_irq;
@@ -1093,11 +1205,17 @@ free_irq:
 free_btemp_wq:
 	destroy_workqueue(di->btemp_wq);
 free_device_info:
-	kfree(di);
+	devm_kfree(&pdev->dev, di);
+err_no_mem:

 	return ret;
 }

+static const struct of_device_id ab8500_btemp_match[] = {
+	{.compatible = "stericsson,ab8500-btemp",},
+	{},
+};
+
 static struct platform_driver ab8500_btemp_driver = {
 	.probe = ab8500_btemp_probe,
 	.remove = __devexit_p(ab8500_btemp_remove),
@@ -1106,6 +1224,7 @@ static struct platform_driver ab8500_btemp_driver = {
 	.driver = {
 		.name = "ab8500-btemp",
 		.owner = THIS_MODULE,
+		.of_match_table = ab8500_btemp_match,
 	},
 };

-- 
1.7.9.5

Re: mfd: Implement devicetree support for AB8500 Btemp

[Arnd Bergmann]

On Monday 10 September 2012, Rajanikanth HV wrote:
> +
> +supplied-to:
> +       This is a logical binding w.r.t power supply event change
> +       across energy-management-module drivers where in the
> +       runtime battery properties are shared along with uevent
> +       notification.
> +       ref: di->btemp_psy.external_power_changed =
> +               ab8500_btemp_external_power_changed;
> +               ab8500_btemp.c
> +
> +       Need for this property:
> +               btemp, fg and charger updates power-supply properties
> +               based on the events listed above.
> +               Event handler invokes power supply change notifier
> +               which in-turn invokes registered power supply class call-back
> +               based on the 'supplied_to' string.
> +               ref:
> +               power_supply_changed_work(..) ./drivers/power/power_supply_core.c
> +
> +       example:
> +       ab8500-btemp {
> +               /* Other enery management module */
> +               supplied-to = "ab8500_chargalg", "ab8500_fg";
> +               num_supplicants = <2>;
> +       };
> +

This looks like you're doing things the opposite way from everyone else.
Normally, each device uses phandles to refer to other objects it depends
on (gpio lines, regulators, clocks, interrupts, ...), rather than listing
things that depend on it.

Can you turn this around to be more like the others?

Note also that device tree identifiers should use '-' as a word separator,
not '_', and that a binding document should specify the exact set of
possible values. If the properties contain strings, please list every
valid string.

> +               thermister-internal-to-battery = <1>;
> +               li_ion_9100_battery = <0>;

Boolean properties should be empty when enabled and not present when
disabled. In this example, one would just write

		thermister-internal-to-battery;


	Arnd

Re: mfd: Implement devicetree support for AB8500 Btemp

[Linus Walleij]

On Mon, Sep 10, 2012 at 7:01 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 10 September 2012, Rajanikanth HV wrote:

>> +               thermister-internal-to-battery = <1>;
>> +               li_ion_9100_battery = <0>;
>
> Boolean properties should be empty when enabled and not present when
> disabled. In this example, one would just write
>
>                 thermister-internal-to-battery;

I also think the proper word is "thermistor":
http://en.wikipedia.org/wiki/Thermistor

Yours,
Linus Walleij

Re: mfd: Implement devicetree support for AB8500 Btemp

[Arnd Bergmann]

On Tuesday 11 September 2012, Rajanikanth HV wrote:
> >> +Supplied-to:
> >> +     This shall be power supply class dependency where in the
> runtime battery
> >> +     properties will be shared across fuel guage and charging
> algorithm driver.
> >
> > I probably don't understand enough of this, but shouldn't the other
> devices
> > that are supplied by this have a reference to this node rather than doing
> > it this way around? Why use strings here instead of phandles?
> 
> This is a logical binding w.r.t power supply event change
> across energy-management-module drivers where in runtime battery
> properties are shared along with uevent notification.
> ref: di->btemp_psy.external_power_
> changed =
>      ab8500_btemp_external_power_changed;
>      ref: ab8500_btemp.c
> 
> Need for this property:
>  btemp, fg and charger updates power-supply properties
>  based on the events listed above.
>  Event handler invokes power supply change notifier
>  which in-turn invokes registered power supply class call-back
>  based on the 'supplied_to' string.
>  ref:
>    power_supply_changed_work(..) ./drivers/power/power_supply_core.c
> 
> In this case how to approach through phandle?
> ============================
> 

Sorry, I really tried, but I cannot make sense of what you wrote
there. Can you try again and describe in full English sentences
how the hardware blocks are connected and what their purpose is?

	Arnd

Re: Implement devicetree support for AB8500 Btemp

[Rajanikanth HV]

On Tuesday 11 September 2012 04:52 PM, Arnd Bergmann wrote:
> On Tuesday 11 September 2012, Rajanikanth HV wrote:
>>>> +Supplied-to:
>>>> +     This shall be power supply class dependency where in the
>> runtime battery
>>>> +     properties will be shared across fuel guage and charging
>> algorithm driver.
>>>
>>> I probably don't understand enough of this, but shouldn't the other
>> devices
>>> that are supplied by this have a reference to this node rather than doing
>>> it this way around? Why use strings here instead of phandles?
>>
>> This is a logical binding w.r.t power supply event change
>> across energy-management-module drivers where in runtime battery
>> properties are shared along with uevent notification.
>> ref: di->btemp_psy.external_power_
>> changed =
>>      ab8500_btemp_external_power_changed;
>>      ref: ab8500_btemp.c
>>
>> Need for this property:
>>  btemp, fg and charger updates power-supply properties
>>  based on the events listed above.
>>  Event handler invokes power supply change notifier
>>  which in-turn invokes registered power supply class call-back
>>  based on the 'supplied_to' string.
>>  ref:
>>    power_supply_changed_work(..) ./drivers/power/power_supply_core.c
>>
>> In this case how to approach through phandle?
>> ============================
>>
> 
> Sorry, I really tried, but I cannot make sense of what you wrote
> there. Can you try again and describe in full English sentences
> how the hardware blocks are connected
Consider: USB charging:
            ______________________
           |                      |
--(Vbus)-->|   USB Charger with   |
           |  Charger FSM (h/w)   |
           |______________________|
                |    |
                |    |(Vbat and other signals)
                |  __|_____
        to      | |        |(GaugeSense
     Charger FSM| | LION   | Signal)     _____________
                | |Battery |----------->|FuelGauge blk|
                | |________|            |{Coulomb Ctr}|
                |       |                -------------
                |   <Thermistor>
                |       |
                |       | (BatCtrl Signal)
                |_______|---------->[Btemp blk]
                        |               |
                      [ADC]             |__Btemp_Low
                                        |__Btemp_Med
                                        |__Btemp_High

Note: Charging algorithm is a logical entity.

 and what their purpose is?
a) Coulomb counter comprises '12bit adc' and an 'N sample
average/accumulation' logic helps to measure battery capacity
Note: The charge and the discharge current of the battery is
        converted to voltage by an external resistor connected
        between GaugeSenseP and GaugeSenseN pins.

b) Battery temperature monitoring comprises a comparator which is
   enabled only by HW (charger state machine) helps to measure
   the thermal threshold
Note: The accuracy of the battery temperature measurement depends
        of the accuracy of the thermistor used.

c) Charger provides 'Constant Current Constant Voltage' USB and
   Main(Wall) Charging support, it embeds: voltage detection,
   thermal protection, Constant voltage charging with programmable level,
   clock dithering and battery voltage monitoring

e.g. Correlation between charger and Btemp
- if the battery temperature is higher than “MaxTemp °C,
  the charger does not start, but is enabled

- if the battery temperature is between 0°C and “MaxTemp” °C
  charging is done in AB8500 Hardware control mode

- charging is done in DB8500 Software control mode, if the battery:
  has a voltage higher than the “BattOK Threshold

- If the battery temperature is between -10°C and 0°C:
  charging is done in AB8500 Hardware control mode

- If the battery temperature is below -10°C, charging is
  done in AB8500 Hardware control mode
> 
> 	Arnd
> 

Re: Implement devicetree support for AB8500 Btemp

[Arnd Bergmann]

On Wednesday 12 September 2012, Rajanikanth HV wrote:
> On Tuesday 11 September 2012 04:52 PM, Arnd Bergmann wrote:
> > On Tuesday 11 September 2012, Rajanikanth HV wrote:

> Consider: USB charging:
>             ______________________
>            |                      |
> --(Vbus)-->|   USB Charger with   |
>            |  Charger FSM (h/w)   |
>            |______________________|
>                 |    |
>                 |    |(Vbat and other signals)
>                 |  __|_____
>         to      | |        |(GaugeSense
>      Charger FSM| | LION   | Signal)     _____________
>                 | |Battery |----------->|FuelGauge blk|
>                 | |________|            |{Coulomb Ctr}|
>                 |       |                -------------
>                 |   <Thermistor>
>                 |       |
>                 |       | (BatCtrl Signal)
>                 |_______|---------->[Btemp blk]
>                         |               |
>                       [ADC]             |__Btemp_Low
>                                         |__Btemp_Med
>                                         |__Btemp_High
> 
> Note: Charging algorithm is a logical entity.
> 
>  and what their purpose is?
> a) Coulomb counter comprises '12bit adc' and an 'N sample
> average/accumulation' logic helps to measure battery capacity
> Note: The charge and the discharge current of the battery is
>         converted to voltage by an external resistor connected
>         between GaugeSenseP and GaugeSenseN pins.
> 
> b) Battery temperature monitoring comprises a comparator which is
>    enabled only by HW (charger state machine) helps to measure
>    the thermal threshold
> Note: The accuracy of the battery temperature measurement depends
>         of the accuracy of the thermistor used.
> 
> c) Charger provides 'Constant Current Constant Voltage' USB and
>    Main(Wall) Charging support, it embeds: voltage detection,
>    thermal protection, Constant voltage charging with programmable level,
>    clock dithering and battery voltage monitoring
> 
> e.g. Correlation between charger and Btemp
> - if the battery temperature is higher than “MaxTemp °C,
>   the charger does not start, but is enabled
> 
> - if the battery temperature is between 0°C and “MaxTemp” °C
>   charging is done in AB8500 Hardware control mode
> 
> - charging is done in DB8500 Software control mode, if the battery:
>   has a voltage higher than the “BattOK Threshold
> 
> - If the battery temperature is between -10°C and 0°C:
>   charging is done in AB8500 Hardware control mode
> 
> - If the battery temperature is below -10°C, charging is
>   done in AB8500 Hardware control mode


Ok, thanks for the explanation, this is starting to make a lot more
sense. So the three blocks (fb, btemp, charger) are all separate
mfd cells, each with their own register set in ab8500 and a separate
driver in drivers/power, right?

Then there is the ab8500-charalg driver which I guess is implementing
the Charger FSM you mention above, but it doesn't have any registers
in the ab8500 but rather ties the other drivers together.

If this is true, I don't understand what makes the 'supplied-to'
properties you list in the device tree binding board specific. Are
they not always done the same way? If so, you could just leave them
out.

What does indeed seem to be needed is a place to identify the battery
type, but it's not clear if the btemp device is the best place for
that (maybe it is). For this, I would suggest you give a list of
possible batteries and require a property such as

 st-ericsson,battery-type: A string identifier for the type of battery,
			   which impacts how an operating system interpret
			   the sensor readings. Possible values include:
	* "none"	-- no battery connected
	* "li-ion-9100" -- Type 9100 Li-ION battery
	* <add any others that apply here>

	Arnd

Re: Implement devicetree support for AB8500 Btemp

[Rajanikanth HV]

On Wednesday 12 September 2012 09:06 PM, Arnd Bergmann wrote:
> On Wednesday 12 September 2012, Rajanikanth HV wrote:
>> On Tuesday 11 September 2012 04:52 PM, Arnd Bergmann wrote:
>>> On Tuesday 11 September 2012, Rajanikanth HV wrote:
> 
>> Consider: USB charging:
>>             ______________________
>>            |                      |
>> --(Vbus)-->|   USB Charger with   |
>>            |  Charger FSM (h/w)   |
>>            |______________________|
>>                 |    |
>>                 |    |(Vbat and other signals)
>>                 |  __|_____
>>         to      | |        |(GaugeSense
>>      Charger FSM| | LION   | Signal)     _____________
>>                 | |Battery |----------->|FuelGauge blk|
>>                 | |________|            |{Coulomb Ctr}|
>>                 |       |                -------------
>>                 |   <Thermistor>
>>                 |       |
>>                 |       | (BatCtrl Signal)
>>                 |_______|---------->[Btemp blk]
>>                         |               |
>>                       [ADC]             |__Btemp_Low
>>                                         |__Btemp_Med
>>                                         |__Btemp_High
>>
>> Note: Charging algorithm is a logical entity.
> Ok, thanks for the explanation, this is starting to make a lot more
> sense. So the three blocks (fb, btemp, charger) are all separate
> mfd cells, each with their own register set in ab8500 and a separate
> driver in drivers/power, right?
Correct, You can have a look at ab8500 spec:
www.stericsson.com/developers/CD00291561_UM1031_AB8500_user_manual-rev5_CTDS_public.pdf
> 
> Then there is the ab8500-charalg driver which I guess is implementing
> the Charger FSM you mention above,
ab8500-charalg does not implements charger FSM, Charger FSM is a
hardware block for which any functional info is not available in the
spec. However, ab8500-charalg implements state m/c depicted in the
figure (9) of spec, implementation can be found in the code:
abx500_chargalg.c: centered around abx500_chargalg_algorithm(..)

Let me brief out what 'charger(Wall/USB)' and 'chargalg' driver does:

(a) Charger driver implements:
	- events specific to Wall(a/c) and USB Charger
	- power supply attributes handling and notification
	  to power_supply core. Ref: enum power_supply_property
	  linux/power_supply.h
	  Note: subset of power_supply_property are handled	
	- turn on/off charging led, AC and USB charging
	- Regulating Voltage and Current values for charging
	- voltage threshold check
	- Charging regulation based on btemp
all this functionality has register dependency

(b) Charging algorithm:
	- Starts by collecting power_supply properties across power
          class devices which are 'bm' devices
	- Maintains the different charging states across ac and usb
	  charging process pertaining to 'Vbus, main or Vbat', thermal,
	  btemp etc.,	
	- Implements subset of power_suppply_class properties	
Note: Do not have direct register interface

 but it doesn't have any registers
yes, but make use of power supply properties updated by other bm devs
while managing charging states.
> in the ab8500 but rather ties the other drivers together.
> 
> If this is true, I don't understand what makes the 'supplied-to'
> properties you list in the device tree binding board specific. Are
> they not always done the same way? If so, you could just leave them
> out.
Precisely 'supplied-to' is not board specific, it was maintained as
platform_data which i migrated to dt-node. It is meant to establish
dependency across bm drivers based on power_supply property and
runtime battery attributes.
Basically, 'supplied-to' provides a way of exporting change in
power_supply_property and runtime batter characteristics so that other
bm devs shall make use or refer the updated values.
Ref: external_power_changed(...) call back api.
Note: all the bm drivers handles subset of power_supply property and
      battery attributes,
      ref: include/linux/power_supply.h and get_property(...) call back
      api across bm drivers.
> 
> What does indeed seem to be needed is a place to identify the battery
> type, but it's not clear if the btemp device is the best place for
> that (maybe it is). 
I am not clear whether you are trying to correlate battery-type with
supplied-to. however, battery type is identified based on the
resistance value measured at batctrl pin which is expected to be in the
allowable limit of ab8500 device. This resistance limit varies across
battery types. This happens in btemp driver.

For this, I would suggest you give a list of
> possible batteries and require a property such as
> 
>  st-ericsson,battery-type: A string identifier for the type of battery,
> 			   which impacts how an operating system interpret
> 			   the sensor readings. Possible values include:
> 	* "none"	-- no battery connected
> 	* "li-ion-9100" -- Type 9100 Li-ION battery
> 	* <add any others that apply here>
Can do this, not precisely as "st-ericsson,battery-type", it will be as
battery-type = [unknown|NiMH|LION|...|]], reason being
allowable battery type is based on technology, as you can see the
possible types as:
POWER_SUPPLY_TECHNOLOGY_UNKNOWN = 0,
        POWER_SUPPLY_TECHNOLOGY_NiMH,
        POWER_SUPPLY_TECHNOLOGY_LION,
        POWER_SUPPLY_TECHNOLOGY_LIPO,
        POWER_SUPPLY_TECHNOLOGY_LiFe,
        POWER_SUPPLY_TECHNOLOGY_NiCd,
        POWER_SUPPLY_TECHNOLOGY_LiMn
Ref: include/linux/power_supply.h
Note: doing this will impact my of_probe(...), may slightly bloat the
code.
> 
> 	Arnd
> 

Re: Implement devicetree support for AB8500 Btemp

[Arnd Bergmann]

On Thursday 13 September 2012, Rajanikanth HV wrote:
> On Wednesday 12 September 2012 09:06 PM, Arnd Bergmann wrote:> > 
> > If this is true, I don't understand what makes the 'supplied-to'
> > properties you list in the device tree binding board specific. Are
> > they not always done the same way? If so, you could just leave them
> > out.
> Precisely 'supplied-to' is not board specific, it was maintained as
> platform_data which i migrated to dt-node. It is meant to establish
> dependency across bm drivers based on power_supply property and
> runtime battery attributes.
> Basically, 'supplied-to' provides a way of exporting change in
> power_supply_property and runtime batter characteristics so that other
> bm devs shall make use or refer the updated values.
> Ref: external_power_changed(...) call back api.
> Note: all the bm drivers handles subset of power_supply property and
>       battery attributes,
>       ref: include/linux/power_supply.h and get_property(...) call back
>       api across bm drivers.

Ok, so you want to just remove the property from the device tree,
or do you want to establish a different method to specify these
connections?

> > What does indeed seem to be needed is a place to identify the battery
> > type, but it's not clear if the btemp device is the best place for
> > that (maybe it is). 
> I am not clear whether you are trying to correlate battery-type with
> supplied-to. however, battery type is identified based on the
> resistance value measured at batctrl pin which is expected to be in the
> allowable limit of ab8500 device. This resistance limit varies across
> battery types. This happens in btemp driver.

I wasn't correlating them. I just mentioned that unlike the supplied-to
property, the battery type property does seem to belong into the device
tree.

> For this, I would suggest you give a list of
> > possible batteries and require a property such as
> > 
> >  st-ericsson,battery-type: A string identifier for the type of battery,
> > 			   which impacts how an operating system interpret
> > 			   the sensor readings. Possible values include:
> > 	* "none"	-- no battery connected
> > 	* "li-ion-9100" -- Type 9100 Li-ION battery
> > 	* <add any others that apply here>
> Can do this, not precisely as "st-ericsson,battery-type", it will be as
> battery-type = [unknown|NiMH|LION|...|]], reason being
> allowable battery type is based on technology, as you can see the
> possible types as:
> POWER_SUPPLY_TECHNOLOGY_UNKNOWN = 0,
>         POWER_SUPPLY_TECHNOLOGY_NiMH,
>         POWER_SUPPLY_TECHNOLOGY_LION,
>         POWER_SUPPLY_TECHNOLOGY_LIPO,
>         POWER_SUPPLY_TECHNOLOGY_LiFe,
>         POWER_SUPPLY_TECHNOLOGY_NiCd,
>         POWER_SUPPLY_TECHNOLOGY_LiMn
> Ref: include/linux/power_supply.h
> Note: doing this will impact my of_probe(...), may slightly bloat the
> code.

Ok.

If you want to make the battery type a generic property, it's probably
best to start a separate binding document for this in
Documentation/devicetree/bindings/power-supply/common.txt
and document a string for each of these.

If we expect the property to be needed only for ab8500, please use
a vendor prefix like 'stericsson,'.

	Arnd

Re: Implement devicetree support for AB8500 Btemp

[Anton Vorontsov]

(Thanks for Cc'ing me.)

On Thu, Sep 13, 2012 at 02:37:38PM +0000, Arnd Bergmann wrote:
[...]
> > > If this is true, I don't understand what makes the 'supplied-to'
> > > properties you list in the device tree binding board specific. Are
> > > they not always done the same way? If so, you could just leave them
> > > out.
> > Precisely 'supplied-to' is not board specific, it was maintained as
> > platform_data which i migrated to dt-node. It is meant to establish
> > dependency across bm drivers based on power_supply property and
> > runtime battery attributes.
> > Basically, 'supplied-to' provides a way of exporting change in
> > power_supply_property and runtime batter characteristics so that other
> > bm devs shall make use or refer the updated values.
> > Ref: external_power_changed(...) call back api.
> > Note: all the bm drivers handles subset of power_supply property and
> >       battery attributes,
> >       ref: include/linux/power_supply.h and get_property(...) call back
> >       api across bm drivers.
> 
> Ok, so you want to just remove the property from the device tree,
> or do you want to establish a different method to specify these
> connections?

Power supply subsystem's supplied_to describes not just how driver
should notify other devices, supplied_to is more generic stuff, in terms
that it describes power supply hierarchy. It's like a directed graph,
e.g.:

      <AC power> supplied_to <main battery> and <backup battery>
     <USB power> supplied_to <main battery> and <backup battery>
  <main battery> supplied_to <system>
<backup battery> supplied_to <system>
  <cmos battery> supplied_to <southbridge pci device>
  <mice battery> supplied_to <mice wireless hid>

How things interact in linux are just implementations details.
So, device tree is surely a perfect place to describe these things.

Although, in current bindings I see this:

+       ab8500-fg {
+               /* Other enery management module */
+               supplied_to = "ab8500_chargalg", "ab8500_usb";
+               num_supplicants = <2>;
+       };

Instead of addressing supplicants by name, it's better to address
via phandles. And, of course, num_supplicants is not needed, it can
be derived.

[...]
> > > possible batteries and require a property such as
> > > 
> > >  st-ericsson,battery-type: A string identifier for the type of battery,
> > > 			   which impacts how an operating system interpret
> > > 			   the sensor readings. Possible values include:
> > > 	* "none"	-- no battery connected
> > > 	* "li-ion-9100" -- Type 9100 Li-ION battery
> > > 	* <add any others that apply here>
> > Can do this, not precisely as "st-ericsson,battery-type", it will be as
> > battery-type = [unknown|NiMH|LION|...|]], reason being
> > allowable battery type is based on technology, as you can see the
> > possible types as:
> > POWER_SUPPLY_TECHNOLOGY_UNKNOWN = 0,
> >         POWER_SUPPLY_TECHNOLOGY_NiMH,
> >         POWER_SUPPLY_TECHNOLOGY_LION,
> >         POWER_SUPPLY_TECHNOLOGY_LIPO,
> >         POWER_SUPPLY_TECHNOLOGY_LiFe,
> >         POWER_SUPPLY_TECHNOLOGY_NiCd,
> >         POWER_SUPPLY_TECHNOLOGY_LiMn
> > Ref: include/linux/power_supply.h
> > Note: doing this will impact my of_probe(...), may slightly bloat the
> > code.
> 
> Ok.
> 
> If you want to make the battery type a generic property, it's probably
> best to start a separate binding document for this in
> Documentation/devicetree/bindings/power-supply/common.txt
> and document a string for each of these.

Fully agree. We need to document generic DT bindings for power supplies.

Thanks,
Anton.

Re: Implement devicetree support for AB8500 Btemp

[Arnd Bergmann]

On Friday 14 September 2012, Anton Vorontsov wrote:
> Power supply subsystem's supplied_to describes not just how driver
> should notify other devices, supplied_to is more generic stuff, in terms
> that it describes power supply hierarchy. It's like a directed graph,
> e.g.:
> 
>       <AC power> supplied_to <main battery> and <backup battery>
>      <USB power> supplied_to <main battery> and <backup battery>
>   <main battery> supplied_to <system>
> <backup battery> supplied_to <system>
>   <cmos battery> supplied_to <southbridge pci device>
>   <mice battery> supplied_to <mice wireless hid>
> 
> How things interact in linux are just implementations details.
> So, device tree is surely a perfect place to describe these things.
> 
> Although, in current bindings I see this:
> 
> +       ab8500-fg {
> +               /* Other enery management module */
> +               supplied_to = "ab8500_chargalg", "ab8500_usb";
> +               num_supplicants = <2>;
> +       };
> 
> Instead of addressing supplicants by name, it's better to address
> via phandles. And, of course, num_supplicants is not needed, it can
> be derived.

Right. that's what I thought. The other comment I made initially is
that it would be more in the spirit of the existing bindings to have
the supply property in the opposite directory, if we need it, like
(picking up your above example):


/ {
	/* power supply property in the root node is used by default */
	power-supply = <&main-battery>, <&backup-battery>;

	ac-power: power@... {
		...
	};

	usb-power: power@... {
		...
	};

	main-battery: battery@... {
		power-supply = <&ac-power>, <&usb-power};
	;

	...
};

It's the same information and absolutely equivalent as far as I can tell,
but it feel more logical in the way we tend to describe things.

	Arnd

Re: Implement devicetree support for AB8500 Btemp

[Rajanikanth HV]

On Friday 14 September 2012 01:39 PM, Arnd Bergmann wrote:
> On Friday 14 September 2012, Anton Vorontsov wrote:
>> Power supply subsystem's supplied_to describes not just how driver
>> should notify other devices, supplied_to is more generic stuff, in terms
>> that it describes power supply hierarchy. It's like a directed graph,
>> e.g.:
>>
>>       <AC power> supplied_to <main battery> and <backup battery>
>>      <USB power> supplied_to <main battery> and <backup battery>
>>   <main battery> supplied_to <system>
>> <backup battery> supplied_to <system>
>>   <cmos battery> supplied_to <southbridge pci device>
>>   <mice battery> supplied_to <mice wireless hid>
>>
>> How things interact in linux are just implementations details.
>> So, device tree is surely a perfect place to describe these things.
>>
>> Although, in current bindings I see this:
>>
>> +       ab8500-fg {
>> +               /* Other enery management module */
>> +               supplied_to = "ab8500_chargalg", "ab8500_usb";
>> +               num_supplicants = <2>;
>> +       };
>>
>> Instead of addressing supplicants by name, it's better to address
>> via phandles. And, of course, num_supplicants is not needed, it can
>> be derived.
> 
> Right. that's what I thought. The other comment I made initially is
> that it would be more in the spirit of the existing bindings to have
> the supply property in the opposite directory, if we need it, like
> (picking up your above example):
> 
> 
> / {
> 	/* power supply property in the root node is used by default */
> 	power-supply = <&main-battery>, <&backup-battery>;
> 
> 	ac-power: power@... {
> 		...
> 	};
> 
> 	usb-power: power@... {
> 		...
> 	};
> 
> 	main-battery: battery@... {
> 		power-supply = <&ac-power>, <&usb-power};
> 	;
> 
> 	...
> };
> 
> It's the same information and absolutely equivalent as far as I can tell,
> but it feel more logical in the way we tend to describe things.
> 
> 	Arnd
> 
phandle'd supplied-to will looks like:

usb: ab8500-usb {

};

battery : ab8500-bat-type {
    battery-name = "unknown|NiMH|LION|LIPO|LiFe|NiCd|LiMn";
    thermistor_on_batctrl = <1>;
};

chargalg: ab8500-chargalg {
    compatible = "stericsson,ab8500-chargalg";
    interface-name = "ab8500_chargalg";
    battery-info = <&ab8500-bat-type>
    supplied-to = <&ab8500_fg>;
	...
};

fuelguage: ab8500-fg {
    compatible = "stericsson,ab8500-fg";
    interface-name = "ab8500_fg";
    battery-info = <&ab8500-bat-type>
    supplied-to = <&ab8500_chargalg &ab8500_usb>;
	...
};
btemp: ab8500-btemp {
    compatible = "stericsson,ab8500-fg";
    interface-name = "ab8500_btemp";
    battery-info = <&ab8500-bat-type>
    supplied-to = <&ab8500_chargalg &ab8500_fg>;
	...
};

charger: ab8500-charger {
    compatible = "stericsson,ab8500-charger";
    interface-name = "ab8500_charger";
    battery-info = <&ab8500-bat-type>
    supplied-to = <&ab8500_chargalg &ab8500_fg &ab8500_btemp>;
	...
};

Re: Implement devicetree support for AB8500 Btemp

[Rajanikanth HV]

On Thursday 13 September 2012 08:07 PM, Arnd Bergmann wrote:
> On Thursday 13 September 2012, Rajanikanth HV wrote:
>> On Wednesday 12 September 2012 09:06 PM, Arnd Bergmann wrote:> > 
>>> If this is true, I don't understand what makes the 'supplied-to'
>>> properties you list in the device tree binding board specific. Are
>>> they not always done the same way? If so, you could just leave them
>>> out.
>> Precisely 'supplied-to' is not board specific, it was maintained as
>> platform_data which i migrated to dt-node. It is meant to establish
>> dependency across bm drivers based on power_supply property and
>> runtime battery attributes.
>> Basically, 'supplied-to' provides a way of exporting change in
>> power_supply_property and runtime batter characteristics so that other
>> bm devs shall make use or refer the updated values.
>> Ref: external_power_changed(...) call back api.
>> Note: all the bm drivers handles subset of power_supply property and
>>       battery attributes,
>>       ref: include/linux/power_supply.h and get_property(...) call back
>>       api across bm drivers.
> 
> Ok, so you want to just remove the property from the device tree,
> or do you want to establish a different method to specify these
> connections?
> 
>>> What does indeed seem to be needed is a place to identify the battery
>>> type, but it's not clear if the btemp device is the best place for
>>> that (maybe it is). 
>> I am not clear whether you are trying to correlate battery-type with
>> supplied-to. however, battery type is identified based on the
>> resistance value measured at batctrl pin which is expected to be in the
>> allowable limit of ab8500 device. This resistance limit varies across
>> battery types. This happens in btemp driver.
> 
> I wasn't correlating them. I just mentioned that unlike the supplied-to
> property, the battery type property does seem to belong into the device
> tree.
> 
>> For this, I would suggest you give a list of
>>> possible batteries and require a property such as
>>>
>>>  st-ericsson,battery-type: A string identifier for the type of battery,
>>> 			   which impacts how an operating system interpret
>>> 			   the sensor readings. Possible values include:
>>> 	* "none"	-- no battery connected
>>> 	* "li-ion-9100" -- Type 9100 Li-ION battery
>>> 	* <add any others that apply here>
>> Can do this, not precisely as "st-ericsson,battery-type", it will be as
>> battery-type = [unknown|NiMH|LION|...|]], reason being
>> allowable battery type is based on technology, as you can see the
>> possible types as:
>> POWER_SUPPLY_TECHNOLOGY_UNKNOWN = 0,
>>         POWER_SUPPLY_TECHNOLOGY_NiMH,
>>         POWER_SUPPLY_TECHNOLOGY_LION,
>>         POWER_SUPPLY_TECHNOLOGY_LIPO,
>>         POWER_SUPPLY_TECHNOLOGY_LiFe,
>>         POWER_SUPPLY_TECHNOLOGY_NiCd,
>>         POWER_SUPPLY_TECHNOLOGY_LiMn
>> Ref: include/linux/power_supply.h
>> Note: doing this will impact my of_probe(...), may slightly bloat the
>> code.
> 
> Ok.
> 
> If you want to make the battery type a generic property, it's probably
> best to start a separate binding document for this in
> Documentation/devicetree/bindings/power-supply/common.txt
> and document a string for each of these.
> 
> If we expect the property to be needed only for ab8500, please use
> a vendor prefix like 'stericsson,'.
it is fine to have battery-name in the battery-type property as said
above and to have binding document, the basic requirement is to
maintain the battery type information for a specified battery also
within the same technology of battery method, battery parameters may
vary. Presently battery-type and its dependent information is
maintained in the drivers/power/ folder as per the arnd review
sometimes back.




> 
> 	Arnd
> 

Re: mfd: Implement devicetree support for AB8500 Btemp

[Francesco Lavra]

On 09/10/2012 01:21 PM, Rajanikanth HV wrote:
...
> -static int __devinit ab8500_btemp_probe(struct platform_device *pdev)
> +static int __devinit
> +btemp_of_probe(struct device *dev,
> +		struct device_node *np,
> +		struct abx500_bm_plat_data *bm_pdata)
>  {
> -	int irq, i, ret = 0;
> -	u8 val;
> -	struct abx500_bm_plat_data *plat_data = pdev->dev.platform_data;
> -	struct ab8500_btemp *di;
> -
> -	if (!plat_data) {
> -		dev_err(&pdev->dev, "No platform data\n");
> -		return -EINVAL;
> +	u8	val;
> +	u32	pval;
> +	int	i;
> +	int	ext_thermister, lion_battery, ret = 0;
> +	const char *bm_dev_name;
> +	struct	abx500_btemp_platform_data *btemp = bm_pdata->btemp;
> +	struct	abx500_bm_data		   *bat;
> +	struct	abx500_battery_type	   *btype;
> +
> +	ret = of_property_read_u32(np, "num_supplicants", &pval);
> +	if (ret) {
> +		dev_err(dev, "missing property num_supplicants\n");
> +		ret = -EINVAL;
> +		goto inval_pval;
>  	}
> -
> -	di = kzalloc(sizeof(*di), GFP_KERNEL);
> -	if (!di)
> -		return -ENOMEM;
> -
> -	/* get parent data */
> -	di->dev = &pdev->dev;
> -	di->parent = dev_get_drvdata(pdev->dev.parent);
> -	di->gpadc = ab8500_gpadc_get("ab8500-gpadc.0");
> -
> -	/* get btemp specific platform data */
> -	di->pdata = plat_data->btemp;
> -	if (!di->pdata) {
> -		dev_err(di->dev, "no btemp platform data supplied\n");
> +	btemp->num_supplicants = pval;
> +	btemp->supplied_to =
> +		devm_kzalloc(dev, btemp->num_supplicants *
> +			sizeof(const char *), GFP_KERNEL);
> +	if (btemp->supplied_to == NULL) {
> +		dev_err(dev, "%s no mem for supplied_to\n", __func__);
> +		ret = -ENOMEM;
> +		goto inval_pval;
> +	}
> +	for (val = 0; val < btemp->num_supplicants; ++val)
> +		if (of_property_read_string_index
> +			(np, "supplied_to", val, &bm_dev_name) == 0)
> +			*(btemp->supplied_to + val) = (char *)bm_dev_name;
> +		else {
> +			dev_err(dev, "insufficient number of supplied_to data found\n");
> +			ret = -EINVAL;
> +			goto free_dev_mem;
> +		}
> +	ret = of_property_read_u32(np, "thermister_on_batctrl", &pval);
> +	if (ret) {
> +		dev_err(dev, "missing property thermister_on_batctrl\n");
>  		ret = -EINVAL;
> -		goto free_device_info;
> +		goto free_dev_mem;
> +	}
> +	bm_pdata->battery = &ab8500_bm_data;
> +	bat = bm_pdata->battery;
> +	ext_thermister = 0;
> +	if (pval == 0) {
> +		bat->n_btypes = 4;
> +		bat->bat_type = bat_type_ext_thermister;
> +		bat->adc_therm = ABx500_ADC_THERM_BATTEMP;
> +		ext_thermister = 1;
> +	}
> +	ret = of_property_read_u32(np, "li_ion_9100", &pval);
> +	if (ret) {
> +		dev_err(dev, "missing property li_ion_9100\n");
> +		ret = -EINVAL;
> +		goto free_dev_mem;
> +	}
> +	lion_battery = 0;
> +	if (pval == 1) {
> +		bat->no_maintenance = true;
> +		bat->chg_unknown_bat = true;
> +		bat->bat_type[BATTERY_UNKNOWN].charge_full_design = 2600;
> +		bat->bat_type[BATTERY_UNKNOWN].termination_vol = 4150;
> +		bat->bat_type[BATTERY_UNKNOWN].recharge_vol = 4130;
> +		bat->bat_type[BATTERY_UNKNOWN].normal_cur_lvl = 520;
> +		bat->bat_type[BATTERY_UNKNOWN].normal_vol_lvl = 4200;
> +		lion_battery = 1;
> +	}
> +	/* select the battery resolution table */
> +	for (i = 0; i < bat->n_btypes; ++i) {
> +		btype = (bat->bat_type + i);
> +		if (ext_thermister) {
> +			btype->batres_tbl =
> +				temp_to_batres_tbl_ext_thermister;
> +		} else if (lion_battery) {
> +			btype->batres_tbl =
> +				temp_to_batres_tbl_9100;
> +		} else {
> +			btype->batres_tbl =
> +				temp_to_batres_tbl_thermister;
> +		}
> +	}
> +	return ret;
> +free_dev_mem:
> +	devm_kfree(dev, btemp->supplied_to);
> +inval_pval:
> +	return ret;
> +}
> +
> +static int __devinit ab8500_btemp_probe(struct platform_device *pdev)
> +{
> +	u8	val;
> +	int	i;
> +	int	irq, ret = 0;
> +	struct	abx500_bm_plat_data *pdata = pdev->dev.platform_data;
> +	struct	device_node	*np = pdev->dev.of_node;
> +	struct	ab8500_btemp	*di;
> +
> +	di = devm_kzalloc(&pdev->dev, sizeof(*di), GFP_KERNEL);
> +	if (!di) {
> +		dev_err(&pdev->dev, "%s no mem for ab8500_btemp\n", __func__);
> +		ret = -ENOMEM;
>  	}
> 
> -	/* get battery specific platform data */
> -	di->bat = plat_data->battery;
> -	if (!di->bat) {
> -		dev_err(di->dev, "no battery platform data supplied\n");
> +	if (np) {
> +		if (!pdata) {
> +			pdata =
> +			devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> +			if (!pdata) {
> +				dev_err(&pdev->dev,
> +					"%s no mem for pdata\n", __func__);
> +				ret = -ENOMEM;
> +				goto err_no_mem;
> +			}
> +			pdata->btemp = devm_kzalloc(&pdev->dev,
> +					sizeof(*pdata->btemp), GFP_KERNEL);
> +			if (!pdata->btemp) {
> +				devm_kfree(&pdev->dev, pdata);
> +				dev_err(&pdev->dev,
> +					"%s no mem for pdata->btemp\n",
> +					__func__);
> +				ret = -ENOMEM;
> +				goto free_device_info;
> +			}
> +		}
> +		/* get battery specific platform data */
> +		ret = btemp_of_probe(&pdev->dev, np, pdata);
> +		if (ret) {
> +			devm_kfree(&pdev->dev, pdata->btemp);
> +			devm_kfree(&pdev->dev, pdata);
> +			goto free_device_info;
> +		}
> +	}
> +	if (!pdata) {
> +		dev_err(&pdev->dev,
> +			"%s no btemp platform data found\n", __func__);
>  		ret = -EINVAL;
>  		goto free_device_info;
>  	}
> +	di->pdata = pdata->btemp;
> +	di->bat = pdata->battery;
> +	/* get parent data */
> +	di->dev = &pdev->dev;
> +	di->parent = dev_get_drvdata(pdev->dev.parent);
> +	di->gpadc = ab8500_gpadc_get("ab8500-gpadc.0");
> 
>  	/* BTEMP supply */
>  	di->btemp_psy.name = "ab8500_btemp";
> @@ -1008,7 +1121,6 @@ static int __devinit ab8500_btemp_probe(struct
> platform_device *pdev)
>  	di->btemp_psy.external_power_changed =
>  		ab8500_btemp_external_power_changed;
> 
> -
>  	/* Create a work queue for the btemp */
>  	di->btemp_wq =
>  		create_singlethread_workqueue("ab8500_btemp_wq");
> @@ -1065,7 +1177,7 @@ static int __devinit ab8500_btemp_probe(struct
> platform_device *pdev)
>  			IRQF_SHARED | IRQF_NO_SUSPEND,
>  			ab8500_btemp_irq[i].name, di);
> 
> -		if (ret) {
> +		if (ret < 0) {
>  			dev_err(di->dev, "failed to request %s IRQ %d: %d\n"
>  				, ab8500_btemp_irq[i].name, irq, ret);
>  			goto free_irq;
> @@ -1093,11 +1205,17 @@ free_irq:
>  free_btemp_wq:
>  	destroy_workqueue(di->btemp_wq);
>  free_device_info:
> -	kfree(di);
> +	devm_kfree(&pdev->dev, di);
> +err_no_mem:
> 
>  	return ret;
>  }

Same story as in fg driver, there is no need to call devm_kfree() on
error in probe functions, and kfree(di) should be removed from
ab8500_btemp_remove()

--
Francesco