[1/4] mfd: ab8500: add devicetree support for fuelgauge

Message ID	1349064513-31301-2-git-send-email-rajanikanth.hv@stericsson.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org> From: Rajanikanth H.V <rajanikanth.hv@stericsson.com> To: <lee.jones@linaro.org>, <francescolavra.fl@gmail.com>, <arnd@arndb.de>, <anton.vorontsov@linaro.org>, <linus.walleij@stericsson.com> Subject: [PATCH 1/4] mfd: ab8500: add devicetree support for fuelgauge Date: Mon, 1 Oct 2012 09:38:30 +0530 Message-ID: <1349064513-31301-2-git-send-email-rajanikanth.hv@stericsson.com> In-Reply-To: <1349064513-31301-1-git-send-email-rajanikanth.hv@stericsson.com> References: <1349064513-31301-1-git-send-email-rajanikanth.hv@stericsson.com> MIME-Version: 1.0 summary: Content analysis details: (-4.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [207.126.144.139 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: linaro-dev@lists.linaro.org, patches@linaro.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, STEricsson_nomadik_linux@list.st.com, rajanikanth.hv@stericsson.com Precedence: list Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org

Rajanikanth H.V Oct. 1, 2012, 4:08 a.m. UTC

From: "Rajanikanth H.V" <rajanikanth.hv@stericsson.com>

- This patch adds device tree support for fuelguage driver
- optimize bm devices platform_data usage and of_probe(...)
  Note: of_probe() routine for battery managed devices is made
  common across all bm drivers.

Signed-off-by: Rajanikanth H.V <rajanikanth.hv@stericsson.com>
---
 Documentation/devicetree/bindings/mfd/ab8500.txt   |    8 +-
 .../devicetree/bindings/power_supply/ab8500/fg.txt |   86 +++
 arch/arm/boot/dts/dbx5x0.dtsi                      |   22 +-
 drivers/mfd/ab8500-core.c                          |    1 +
 drivers/power/Makefile                             |    2 +-
 drivers/power/ab8500_bmdata.c                      |  549 ++++++++++++++++++++
 drivers/power/ab8500_btemp.c                       |    4 +-
 drivers/power/ab8500_charger.c                     |    4 +-
 drivers/power/ab8500_fg.c                          |   76 ++-
 drivers/power/abx500_chargalg.c                    |    4 +-
 include/linux/mfd/abx500.h                         |   37 +-
 include/linux/mfd/abx500/ab8500-bm.h               |    7 +
 12 files changed, 744 insertions(+), 56 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power_supply/ab8500/fg.txt
 create mode 100644 drivers/power/ab8500_bmdata.c

Lee Jones Oct. 1, 2012, 9:49 a.m. UTC | #1

On Mon, 01 Oct 2012, Rajanikanth H.V wrote:

> From: "Rajanikanth H.V" <rajanikanth.hv@stericsson.com>
> 
> - This patch adds device tree support for fuelguage driver
> - optimize bm devices platform_data usage and of_probe(...)
>   Note: of_probe() routine for battery managed devices is made
>   common across all bm drivers.
> 
> Signed-off-by: Rajanikanth H.V <rajanikanth.hv@stericsson.com>
> ---
>  Documentation/devicetree/bindings/mfd/ab8500.txt   |    8 +-
>  .../devicetree/bindings/power_supply/ab8500/fg.txt |   86 +++
>  arch/arm/boot/dts/dbx5x0.dtsi                      |   22 +-
>  drivers/mfd/ab8500-core.c                          |    1 +
>  drivers/power/Makefile                             |    2 +-
>  drivers/power/ab8500_bmdata.c                      |  549 ++++++++++++++++++++
>  drivers/power/ab8500_btemp.c                       |    4 +-
>  drivers/power/ab8500_charger.c                     |    4 +-
>  drivers/power/ab8500_fg.c                          |   76 ++-
>  drivers/power/abx500_chargalg.c                    |    4 +-
>  include/linux/mfd/abx500.h                         |   37 +-
>  include/linux/mfd/abx500/ab8500-bm.h               |    7 +
>  12 files changed, 744 insertions(+), 56 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power_supply/ab8500/fg.txt
>  create mode 100644 drivers/power/ab8500_bmdata.c
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ab8500.txt b/Documentation/devicetree/bindings/mfd/ab8500.txt
> index ce83c8d..762dc11 100644
> --- a/Documentation/devicetree/bindings/mfd/ab8500.txt
> +++ b/Documentation/devicetree/bindings/mfd/ab8500.txt
> @@ -24,7 +24,13 @@ ab8500-bm                :                      :              : Battery Manager
>  ab8500-btemp             :                      :              : Battery Temperature
>  ab8500-charger           :                      :              : Battery Charger
>  ab8500-codec             :                      :              : Audio Codec
> -ab8500-fg                :                      :              : Fuel Gauge
> +ab8500-fg                : 			: vddadc       : Fuel Gauge
> +			 : NCONV_ACCU           :	       : Accumulate N Sample Conversion
> +			 : BATT_OVV		:	       : Battery Over Voltage
> +			 : LOW_BAT_F		:	       : LOW threshold battery voltage
> +			 : CC_INT_CALIB		:	       : Counter Counter Internal Calibration

I think you mean: Coulomb Counter.

> +			 : CCEOC		:	       : Coulomb Counter End of Conversion
> +			 :			:	       :

Random empty entry.

>  ab8500-gpadc             : HW_CONV_END          : vddadc       : Analogue to Digital Converter
>                             SW_CONV_END          :              :
>  ab8500-gpio              :                      :              : GPIO Controller
> diff --git a/Documentation/devicetree/bindings/power_supply/ab8500/fg.txt b/Documentation/devicetree/bindings/power_supply/ab8500/fg.txt
> new file mode 100644
> index 0000000..caa33b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power_supply/ab8500/fg.txt
> @@ -0,0 +1,86 @@
> +=== AB8500 Fuel Gauge Driver ===
> +
> +AB8500 is a mixed signal multimedia and power management
> +device comprising: power and energy-management-module,
> +wall-charger, usb-charger, audio codec, general purpose adc,
> +tvout, clock management and sim card interface.
> +
> +Fuel-guage support is part of energy-management-module, the other

Spelling.

> +components of this module are:
> +main-charger, usb-combo-charger and Battery temperature monitoring.
> +
> +The properties below describes the node for fuel guage driver.

Spelling.

> +
> +Required Properties:
> +- compatible = "stericsson,ab8500-fg"
> +- interface-name:
> +	Name of the controller/driver which is part of energy-management-module
> +- supplied-to:

Still not sure about this property, or your justification for use.

> +	This property shall have dependent nodes which represent other
> +	energy-management-module.

Plural?

> +	This is a logical binding w.r.t power supply events

Proper English please, no slang.

> +	across energy-management-module drivers where-in, the

Ill placed comma?

> +	runtime battery properties are shared along with uevent
> +	notification.

Plural?

> +	ref: di->fg.external_power_changed =
> +		ab8500_fg_external_power_changed;
> +		ab8500_fg.c
> +
> +	Need for this property:
> +		energy-management-module driver updates power-supply properties
> +		which are subset of events listed in 'enum power_supply_property',
> +		ref: power_supply.h file
> +		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
> +		di->fg_psy.external_power_changed
> +
> +	example:
> +	ab8500-fg {
> +		/* dependent energy management modules */
> +		supplied-to  = <&ab8500_chargalg &ab8500_usb>;
> +	};
> +
> +	ab8500_battery_info: ab8500_bat_type {
> +		battery-type = <2>;
> +		thermistor-on-batctrl = <1>;

You have this as a bool here, and ...
> +	};
> +
> +Other dependent node for fuel-gauge is:
> +	ab8500_battery_info: ab8500_bat_type {
> +	};
> +	This node will provide information on 'thermistor interface' and
> +	'battery technology type' used.
> +
> +Properties of this node are:
> +thermistor-on-batctrl:
> +	A boolean value indicating thermistor interface	to battery
> +
> +	Note:
> +	'btemp' and 'batctrl' are the pins interfaced for battery temperature
> +	measurement, 'btemp' signal is used when NTC(negative temperature
> +	coefficient) resister is interfaced external to battery whereas
> +	'batctrl' pin is used when NTC resister is internal to battery.
> +
> +	e.g:
> +	ab8500_battery_info: ab8500_bat_type {
> +		thermistor-on-batctrl;

... a standard property here. I suggest you drop the bool value.

> +	};
> +	indiactes: NTC resister is internal to battery, 'batctrl' is used
> +		for thermal measurement.
> +
> +	The absence of property 'thermal-on-batctrl' indicates
> +	NTC resister is external to battery and  'btemp' signal is used
> +	for thermal measurement.
> +
> +battery-type:
> +	This shall be the battery manufacturing technology type,
> +	allowed types are:
> +		"UNKNOWN" "NiMH" "LION" "LIPO" "LiFe" "NiCd" "LiMn"
> +	e.g:
> +	ab8500_battery_info: ab8500_bat_type {
> +		battery-name = "LION";
> +	}
> +
> diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
> index 748ba7a..bd22c56 100644
> --- a/arch/arm/boot/dts/dbx5x0.dtsi
> +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> @@ -352,8 +352,28 @@
>  					vddadc-supply = <&ab8500_ldo_tvout_reg>;
>  				};
>  
> -				ab8500-usb {
> +				ab8500_battery_info: ab8500_bat_type {
> +					battery-name = "LION";

All new properties have to be documented.

Vendor specific properties should be prepended with the vendor name, so
either write a generic binding document for all to use or prefix with
'stericsson,".

> +					thermistor-on-batctrl;
> +				};
> +
> +				ab8500_chargalg: ab8500_chalg {
> +					compatible     = "stericsson,ab8500-chargalg";
> +					interface-name = "ab8500_chargalg";

Same with all of your new properties (I'll stop mentioning them now).

> +					battery-info   = <&ab8500_battery_info>;
> +					supplied-to    = <&ab8500_fuel_gauge>;

Weren't you going to reverse this logic to be more inline with how
the reset of Device Tree works?

> +				};
> +
> +				ab8500_fuel_gauge: ab8500_fg {
> +					compatible     = "stericsson,ab8500-fg";
> +					interface-name = "ab8500_fg";
> +					battery-info   = <&ab8500_battery_info>;
> +					supplied-to    = <&ab8500_chargalg &ab8500_usb>;

As above.

> +				};
> +
> +				ab8500_usb: ab8500_usb_if {

What does 'if' mean?

>  					compatible = "stericsson,ab8500-usb";
> +					interface-name = "ab8500_usb";

Why is this required?

>  					interrupts = < 90 0x4
>  						       96 0x4
>  						       14 0x4
> diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
> index 1667c77..6c3d7c2 100644
> --- a/drivers/mfd/ab8500-core.c
> +++ b/drivers/mfd/ab8500-core.c
> @@ -1051,6 +1051,7 @@ static struct mfd_cell __devinitdata ab8500_bm_devs[] = {
>  	},
>  	{
>  		.name = "ab8500-fg",
> +		.of_compatible = "stericsson,ab8500-fg",
>  		.num_resources = ARRAY_SIZE(ab8500_fg_resources),
>  		.resources = ab8500_fg_resources,
>  	},
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index ee58afb..2c58d4e 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -34,7 +34,7 @@ obj-$(CONFIG_BATTERY_S3C_ADC)	+= s3c_adc_battery.o
>  obj-$(CONFIG_CHARGER_PCF50633)	+= pcf50633-charger.o
>  obj-$(CONFIG_BATTERY_JZ4740)	+= jz4740-battery.o
>  obj-$(CONFIG_BATTERY_INTEL_MID)	+= intel_mid_battery.o
> -obj-$(CONFIG_AB8500_BM)		+= ab8500_charger.o ab8500_btemp.o ab8500_fg.o abx500_chargalg.o
> +obj-$(CONFIG_AB8500_BM)		+= ab8500_bmdata.o ab8500_charger.o ab8500_fg.o ab8500_btemp.o abx500_chargalg.o
>  obj-$(CONFIG_CHARGER_ISP1704)	+= isp1704_charger.o
>  obj-$(CONFIG_CHARGER_MAX8903)	+= max8903_charger.o
>  obj-$(CONFIG_CHARGER_TWL4030)	+= twl4030_charger.o
> diff --git a/drivers/power/ab8500_bmdata.c b/drivers/power/ab8500_bmdata.c
> new file mode 100644
> index 0000000..d0def3b
> --- /dev/null
> +++ b/drivers/power/ab8500_bmdata.c
> @@ -0,0 +1,549 @@
> +#include <linux/export.h>
> +#include <linux/power_supply.h>
> +#include <linux/of.h>
> +#include <linux/mfd/abx500.h>
> +#include <linux/mfd/abx500/ab8500.h>
> +#include <linux/mfd/abx500/ab8500-bm.h>
> +
> +/*
> + * These are the defined batteries that uses a NTC and ID resistor placed
> + * inside of the battery pack.
> + * Note that the res_to_temp table must be strictly sorted by falling resistance
> + * values to work.
> + */
> +static struct abx500_res_to_temp temp_tbl_A_thermistor[] = {
> +	{-5, 53407},
> +	{ 0, 48594},
> +	{ 5, 43804},
> +	{10, 39188},
> +	{15, 34870},
> +	{20, 30933},
> +	{25, 27422},
> +	{30, 24347},
> +	{35, 21694},
> +	{40, 19431},
> +	{45, 17517},
> +	{50, 15908},
> +	{55, 14561},
> +	{60, 13437},
> +	{65, 12500},
> +};
> +static struct abx500_res_to_temp temp_tbl_B_thermistor[] = {
> +	{-5, 165418},
> +	{ 0, 159024},
> +	{ 5, 151921},
> +	{10, 144300},
> +	{15, 136424},
> +	{20, 128565},
> +	{25, 120978},
> +	{30, 113875},
> +	{35, 107397},
> +	{40, 101629},
> +	{45,  96592},
> +	{50,  92253},
> +	{55,  88569},
> +	{60,  85461},
> +	{65,  82869},
> +};
> +static struct abx500_v_to_cap cap_tbl_A_thermistor[] = {
> +	{4171,	100},
> +	{4114,	 95},
> +	{4009,	 83},
> +	{3947,	 74},
> +	{3907,	 67},
> +	{3863,	 59},
> +	{3830,	 56},
> +	{3813,	 53},
> +	{3791,	 46},
> +	{3771,	 33},
> +	{3754,	 25},
> +	{3735,	 20},
> +	{3717,	 17},
> +	{3681,	 13},
> +	{3664,	  8},
> +	{3651,	  6},
> +	{3635,	  5},
> +	{3560,	  3},
> +	{3408,    1},
> +	{3247,	  0},
> +};
> +static struct abx500_v_to_cap cap_tbl_B_thermistor[] = {
> +	{4161,	100},
> +	{4124,	 98},
> +	{4044,	 90},
> +	{4003,	 85},
> +	{3966,	 80},
> +	{3933,	 75},
> +	{3888,	 67},
> +	{3849,	 60},
> +	{3813,	 55},
> +	{3787,	 47},
> +	{3772,	 30},
> +	{3751,	 25},
> +	{3718,	 20},
> +	{3681,	 16},
> +	{3660,	 14},
> +	{3589,	 10},
> +	{3546,	  7},
> +	{3495,	  4},
> +	{3404,	  2},
> +	{3250,	  0},
> +};
> +
> +static struct abx500_v_to_cap cap_tbl[] = {
> +	{4186,	100},
> +	{4163,	 99},
> +	{4114,	 95},
> +	{4068,	 90},
> +	{3990,	 80},
> +	{3926,	 70},
> +	{3898,	 65},
> +	{3866,	 60},
> +	{3833,	 55},
> +	{3812,	 50},
> +	{3787,	 40},
> +	{3768,	 30},
> +	{3747,	 25},
> +	{3730,	 20},
> +	{3705,	 15},
> +	{3699,	 14},
> +	{3684,	 12},
> +	{3672,	  9},
> +	{3657,	  7},
> +	{3638,	  6},
> +	{3556,	  4},
> +	{3424,	  2},
> +	{3317,	  1},
> +	{3094,	  0},
> +};
> +
> +/*
> + * Note that the res_to_temp table must be strictly sorted by falling
> + * resistance values to work.
> + */
> +static struct abx500_res_to_temp temp_tbl[] = {
> +	{-5, 214834},
> +	{ 0, 162943},
> +	{ 5, 124820},
> +	{10,  96520},
> +	{15,  75306},
> +	{20,  59254},
> +	{25,  47000},
> +	{30,  37566},
> +	{35,  30245},
> +	{40,  24520},
> +	{45,  20010},
> +	{50,  16432},
> +	{55,  13576},
> +	{60,  11280},
> +	{65,   9425},
> +};
> +
> +/*
> + * Note that the batres_vs_temp table must be strictly sorted by falling
> + * temperature values to work.
> + */
> +struct batres_vs_temp temp_to_batres_tbl_thermistor[] = {
> +	{ 40, 120},
> +	{ 30, 135},
> +	{ 20, 165},
> +	{ 10, 230},
> +	{ 00, 325},
> +	{-10, 445},
> +	{-20, 595},
> +};
> +
> +/*
> + * Note that the batres_vs_temp table must be strictly sorted by falling
> + * temperature values to work.
> + */
> +struct batres_vs_temp temp_to_batres_tbl_ext_thermistor[] = {
> +	{ 60, 300},
> +	{ 30, 300},
> +	{ 20, 300},
> +	{ 10, 300},
> +	{ 00, 300},
> +	{-10, 300},
> +	{-20, 300},
> +};
> +
> +/* battery resistance table for LI ION 9100 battery */
> +struct batres_vs_temp temp_to_batres_tbl_9100[] = {
> +	{ 60, 180},
> +	{ 30, 180},
> +	{ 20, 180},
> +	{ 10, 180},
> +	{ 00, 180},
> +	{-10, 180},
> +	{-20, 180},
> +};
> +
> +struct abx500_battery_type bat_type_thermistor[] = {
> +[BATTERY_UNKNOWN] = {
> +	/* First element always represent the UNKNOWN battery */
> +	.name = POWER_SUPPLY_TECHNOLOGY_UNKNOWN,
> +	.resis_high = 0,
> +	.resis_low = 0,
> +	.battery_resistance = 300,
> +	.charge_full_design = 612,
> +	.nominal_voltage = 3700,
> +	.termination_vol = 4050,
> +	.termination_curr = 200,
> +	.recharge_vol = 3990,
> +	.normal_cur_lvl = 400,
> +	.normal_vol_lvl = 4100,
> +	.maint_a_cur_lvl = 400,
> +	.maint_a_vol_lvl = 4050,
> +	.maint_a_chg_timer_h = 60,
> +	.maint_b_cur_lvl = 400,
> +	.maint_b_vol_lvl = 4000,
> +	.maint_b_chg_timer_h = 200,
> +	.low_high_cur_lvl = 300,
> +	.low_high_vol_lvl = 4000,
> +	.n_temp_tbl_elements = ARRAY_SIZE(temp_tbl),
> +	.r_to_t_tbl = temp_tbl,
> +	.n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl),
> +	.v_to_cap_tbl = cap_tbl,
> +	.n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
> +	.batres_tbl = temp_to_batres_tbl_thermistor,
> +},
> +{
> +	.name = POWER_SUPPLY_TECHNOLOGY_LIPO,
> +	.resis_high = 53407,
> +	.resis_low = 12500,
> +	.battery_resistance = 300,
> +	.charge_full_design = 900,
> +	.nominal_voltage = 3600,
> +	.termination_vol = 4150,
> +	.termination_curr = 80,
> +	.recharge_vol = 4130,
> +	.normal_cur_lvl = 700,
> +	.normal_vol_lvl = 4200,
> +	.maint_a_cur_lvl = 600,
> +	.maint_a_vol_lvl = 4150,
> +	.maint_a_chg_timer_h = 60,
> +	.maint_b_cur_lvl = 600,
> +	.maint_b_vol_lvl = 4100,
> +	.maint_b_chg_timer_h = 200,
> +	.low_high_cur_lvl = 300,
> +	.low_high_vol_lvl = 4000,
> +	.n_temp_tbl_elements = ARRAY_SIZE(temp_tbl_A_thermistor),
> +	.r_to_t_tbl = temp_tbl_A_thermistor,
> +	.n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl_A_thermistor),
> +	.v_to_cap_tbl = cap_tbl_A_thermistor,
> +	.n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
> +	.batres_tbl = temp_to_batres_tbl_thermistor,
> +
> +},
> +{
> +	.name = POWER_SUPPLY_TECHNOLOGY_LIPO,
> +	.resis_high = 165418,
> +	.resis_low = 82869,
> +	.battery_resistance = 300,
> +	.charge_full_design = 900,
> +	.nominal_voltage = 3600,
> +	.termination_vol = 4150,
> +	.termination_curr = 80,
> +	.recharge_vol = 4130,
> +	.normal_cur_lvl = 700,
> +	.normal_vol_lvl = 4200,
> +	.maint_a_cur_lvl = 600,
> +	.maint_a_vol_lvl = 4150,
> +	.maint_a_chg_timer_h = 60,
> +	.maint_b_cur_lvl = 600,
> +	.maint_b_vol_lvl = 4100,
> +	.maint_b_chg_timer_h = 200,
> +	.low_high_cur_lvl = 300,
> +	.low_high_vol_lvl = 4000,
> +	.n_temp_tbl_elements = ARRAY_SIZE(temp_tbl_B_thermistor),
> +	.r_to_t_tbl = temp_tbl_B_thermistor,
> +	.n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl_B_thermistor),
> +	.v_to_cap_tbl = cap_tbl_B_thermistor,
> +	.n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
> +	.batres_tbl = temp_to_batres_tbl_thermistor,
> +},
> +};
> +
> +struct abx500_battery_type bat_type_ext_thermistor[] = {
> +[BATTERY_UNKNOWN] = {
> +	/* First element always represent the UNKNOWN battery */
> +	.name = POWER_SUPPLY_TECHNOLOGY_UNKNOWN,
> +	.resis_high = 0,
> +	.resis_low = 0,
> +	.battery_resistance = 300,
> +	.charge_full_design = 612,
> +	.nominal_voltage = 3700,
> +	.termination_vol = 4050,
> +	.termination_curr = 200,
> +	.recharge_vol = 3990,
> +	.normal_cur_lvl = 400,
> +	.normal_vol_lvl = 4100,
> +	.maint_a_cur_lvl = 400,
> +	.maint_a_vol_lvl = 4050,
> +	.maint_a_chg_timer_h = 60,
> +	.maint_b_cur_lvl = 400,
> +	.maint_b_vol_lvl = 4000,
> +	.maint_b_chg_timer_h = 200,
> +	.low_high_cur_lvl = 300,
> +	.low_high_vol_lvl = 4000,
> +	.n_temp_tbl_elements = ARRAY_SIZE(temp_tbl),
> +	.r_to_t_tbl = temp_tbl,
> +	.n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl),
> +	.v_to_cap_tbl = cap_tbl,
> +	.n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
> +	.batres_tbl = temp_to_batres_tbl_thermistor,
> +},
> +/*
> + * These are the batteries that doesn't have an internal NTC resistor to measure
> + * its temperature. The temperature in this case is measure with a NTC placed
> + * near the battery but on the PCB.
> + */
> +{
> +	.name = POWER_SUPPLY_TECHNOLOGY_LIPO,
> +	.resis_high = 76000,
> +	.resis_low = 53000,
> +	.battery_resistance = 300,
> +	.charge_full_design = 900,
> +	.nominal_voltage = 3700,
> +	.termination_vol = 4150,
> +	.termination_curr = 100,
> +	.recharge_vol = 4130,
> +	.normal_cur_lvl = 700,
> +	.normal_vol_lvl = 4200,
> +	.maint_a_cur_lvl = 600,
> +	.maint_a_vol_lvl = 4150,
> +	.maint_a_chg_timer_h = 60,
> +	.maint_b_cur_lvl = 600,
> +	.maint_b_vol_lvl = 4100,
> +	.maint_b_chg_timer_h = 200,
> +	.low_high_cur_lvl = 300,
> +	.low_high_vol_lvl = 4000,
> +	.n_temp_tbl_elements = ARRAY_SIZE(temp_tbl),
> +	.r_to_t_tbl = temp_tbl,
> +	.n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl),
> +	.v_to_cap_tbl = cap_tbl,
> +	.n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
> +	.batres_tbl = temp_to_batres_tbl_thermistor,
> +},
> +{
> +	.name = POWER_SUPPLY_TECHNOLOGY_LION,
> +	.resis_high = 30000,
> +	.resis_low = 10000,
> +	.battery_resistance = 300,
> +	.charge_full_design = 950,
> +	.nominal_voltage = 3700,
> +	.termination_vol = 4150,
> +	.termination_curr = 100,
> +	.recharge_vol = 4130,
> +	.normal_cur_lvl = 700,
> +	.normal_vol_lvl = 4200,
> +	.maint_a_cur_lvl = 600,
> +	.maint_a_vol_lvl = 4150,
> +	.maint_a_chg_timer_h = 60,
> +	.maint_b_cur_lvl = 600,
> +	.maint_b_vol_lvl = 4100,
> +	.maint_b_chg_timer_h = 200,
> +	.low_high_cur_lvl = 300,
> +	.low_high_vol_lvl = 4000,
> +	.n_temp_tbl_elements = ARRAY_SIZE(temp_tbl),
> +	.r_to_t_tbl = temp_tbl,
> +	.n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl),
> +	.v_to_cap_tbl = cap_tbl,
> +	.n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
> +	.batres_tbl = temp_to_batres_tbl_thermistor,
> +},
> +{
> +	.name = POWER_SUPPLY_TECHNOLOGY_LION,
> +	.resis_high = 95000,
> +	.resis_low = 76001,
> +	.battery_resistance = 300,
> +	.charge_full_design = 950,
> +	.nominal_voltage = 3700,
> +	.termination_vol = 4150,
> +	.termination_curr = 100,
> +	.recharge_vol = 4130,
> +	.normal_cur_lvl = 700,
> +	.normal_vol_lvl = 4200,
> +	.maint_a_cur_lvl = 600,
> +	.maint_a_vol_lvl = 4150,
> +	.maint_a_chg_timer_h = 60,
> +	.maint_b_cur_lvl = 600,
> +	.maint_b_vol_lvl = 4100,
> +	.maint_b_chg_timer_h = 200,
> +	.low_high_cur_lvl = 300,
> +	.low_high_vol_lvl = 4000,
> +	.n_temp_tbl_elements = ARRAY_SIZE(temp_tbl),
> +	.r_to_t_tbl = temp_tbl,
> +	.n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl),
> +	.v_to_cap_tbl = cap_tbl,
> +	.n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
> +	.batres_tbl = temp_to_batres_tbl_thermistor,
> +},
> +};
> +
> +static const struct abx500_bm_capacity_levels cap_levels = {
> +	.critical	= 2,
> +	.low		= 10,
> +	.normal		= 70,
> +	.high		= 95,
> +	.full		= 100,
> +};
> +
> +static const struct abx500_fg_parameters fg = {
> +	.recovery_sleep_timer = 10,
> +	.recovery_total_time = 100,
> +	.init_timer = 1,
> +	.init_discard_time = 5,
> +	.init_total_time = 40,
> +	.high_curr_time = 60,
> +	.accu_charging = 30,
> +	.accu_high_curr = 30,
> +	.high_curr_threshold = 50,
> +	.lowbat_threshold = 3100,
> +	.battok_falling_th_sel0 = 2860,
> +	.battok_raising_th_sel1 = 2860,
> +	.user_cap_limit = 15,
> +	.maint_thres = 97,
> +};
> +
> +static const struct abx500_maxim_parameters maxi_params = {
> +	.ena_maxi = true,
> +	.chg_curr = 910,
> +	.wait_cycles = 10,
> +	.charger_curr_step = 100,
> +};
> +
> +static const struct abx500_bm_charger_parameters chg = {
> +	.usb_volt_max		= 5500,
> +	.usb_curr_max		= 1500,
> +	.ac_volt_max		= 7500,
> +	.ac_curr_max		= 1500,
> +};
> +
> +struct abx500_bm_data ab8500_bm_data = {
> +	.temp_under		= 3,
> +	.temp_low		= 8,
> +	.temp_high		= 43,
> +	.temp_over		= 48,
> +	.main_safety_tmr_h	= 4,
> +	.temp_interval_chg	= 20,
> +	.temp_interval_nochg	= 120,
> +	.usb_safety_tmr_h	= 4,
> +	.bkup_bat_v		= BUP_VCH_SEL_2P6V,
> +	.bkup_bat_i		= BUP_ICH_SEL_150UA,
> +	.no_maintenance		= false,
> +	.adc_therm		= ABx500_ADC_THERM_BATCTRL,
> +	.chg_unknown_bat	= false,
> +	.enable_overshoot	= false,
> +	.fg_res			= 100,
> +	.cap_levels		= &cap_levels,
> +	.bat_type		= bat_type_thermistor,
> +	.n_btypes		= 3,
> +	.batt_id		= 0,
> +	.interval_charging	= 5,
> +	.interval_not_charging	= 120,
> +	.temp_hysteresis	= 3,
> +	.gnd_lift_resistance	= 34,
> +	.maxi			= &maxi_params,
> +	.chg_params		= &chg,
> +	.fg_params		= &fg,
> +};
> +
> +int __devinit
> +bmdevs_of_probe(struct device *dev,
> +		struct device_node *np,
> +		struct abx500_bm_plat_data *pdata)
> +{
> +	int	i, ret = 0, thermistor = NTC_INTERNAL;
> +	const	__be32 *ph;
> +	const	char *bat_tech;
> +	struct	abx500_bm_data		 *bat;
> +	struct	abx500_battery_type	 *btype;
> +	struct  device_node		 *np_bat_supply;
> +	struct  abx500_bmdevs_plat_data  *plat_data = pdata->bmdev_pdata;

<nit> 

This spacing is uncharacteristic of Linux drivers. 

Usually, struct declarations come first.

</nit>

> +	/* get phandle to 'supplied-to' node */

I thought you were going to reverse this?

> +	ph = of_get_property(np, "supplied-to", &plat_data->num_supplicants);
> +	if (ph == NULL) {

if (!ph) {

> +		dev_err(dev, "no supplied_to property specified\n");
> +		return -EINVAL;
> +	}
> +	plat_data->num_supplicants /= sizeof(int);
> +	plat_data->supplied_to =
> +		devm_kzalloc(dev, plat_data->num_supplicants *
> +			sizeof(const char *), GFP_KERNEL);
> +	if (plat_data->supplied_to == NULL) {
> +		dev_err(dev, "%s no mem for supplied-to\n", __func__);
> +		return -ENOMEM;
> +	}
> +	for (i = 0; i < plat_data->num_supplicants; ++i) {
> +		np_bat_supply = of_find_node_by_phandle(be32_to_cpup(ph) + i);

Use: of_parse_phandle(np, "supplied-to", i) instead.

> +		if (np_bat_supply == NULL) {

if (!np_bat_supply) {

> +			dev_err(dev, "invalid supplied_to property\n");
> +			return -EINVAL;
> +		}
> +		ret = of_property_read_string(np_bat_supply, "interface-name",
> +				(const char **)(plat_data->supplied_to + i));
> +		if (ret < 0) {
> +			of_node_put(np_bat_supply);
> +			dev_err(dev, "supply/interface name not found\n");
> +			return ret;
> +		}
> +		dev_dbg(dev, "%s power supply interface_name:%s\n",
> +			__func__, *(plat_data->supplied_to + i));
> +	}

<remove>

> +	/* get phandle to 'battery-info' node */
> +	ph = of_get_property(np, "battery-info", NULL);
> +	if (ph == NULL) {
> +		dev_err(dev, "missing property battery-info\n");
> +		return -EINVAL;
> +	}
> +	np_bat_supply = of_find_node_by_phandle(be32_to_cpup(ph));

</remove>

... and replace with: np_bat_supply = of_parse_phandle(np, "battery-info", 0) instead.

> +	if (np_bat_supply == NULL) {

if (!np_bat_supply) {

I'll not mention this again.

> +		dev_err(dev, "invalid battery-info node\n");
> +		return -EINVAL;
> +	}
> +	if (of_property_read_bool(np_bat_supply,
> +			"thermistor-on-batctrl") == false){

Replace with: 
        if (of_get_property(np_bat_supply, "thermistor-on-batctr", NULL))
	        np_bat_supply =  true;

<remove>

> +		dev_warn(dev, "missing property thermistor-on-batctrl\n");
> +		thermistor = NTC_EXTERNAL;
> +	}

</remove>

> +	pdata->battery = &ab8500_bm_data;
> +	bat = pdata->battery;

Why not: bat = &ab8500_bm_data

Or just use ab8500_bm_data in its own right?

> +	if (thermistor == NTC_EXTERNAL) {
> +		bat->n_btypes  = 4;
> +		bat->bat_type  = bat_type_ext_thermistor;
> +		bat->adc_therm = ABx500_ADC_THERM_BATTEMP;
> +	}
> +	ret = of_property_read_string(np_bat_supply, "battery-name", &bat_tech);
> +	if (ret < 0) {
> +		dev_warn(dev, "missing property battery-name/type\n");
> +		bat_tech = "UNKNOWN";
> +	}
> +	of_node_put(np_bat_supply);
> +	if (strcmp(bat_tech, "LION") == 0) {
> +		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;
> +	}
> +	/* select the battery resolution table */
> +	for (i = 0; i < bat->n_btypes; ++i) {
> +		btype = (bat->bat_type + i);
> +		if (thermistor == NTC_EXTERNAL) {
> +			btype->batres_tbl =
> +				temp_to_batres_tbl_ext_thermistor;
> +		} else if (strcmp(bat_tech, "LION") == 0) {

Isn't strncmp safer, since you know the size of the comparison?

> +			btype->batres_tbl =
> +				temp_to_batres_tbl_9100;
> +		} else {
> +			btype->batres_tbl =
> +				temp_to_batres_tbl_thermistor;
> +		}
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL(bmdevs_of_probe);

Why are you exporting?

> diff --git a/drivers/power/ab8500_btemp.c b/drivers/power/ab8500_btemp.c
> index bba3cca..8e427e7 100644
> --- a/drivers/power/ab8500_btemp.c
> +++ b/drivers/power/ab8500_btemp.c
> @@ -93,7 +93,7 @@ struct ab8500_btemp {
>  	struct ab8500 *parent;
>  	struct ab8500_gpadc *gpadc;
>  	struct ab8500_fg *fg;
> -	struct abx500_btemp_platform_data *pdata;
> +	struct abx500_bmdevs_plat_data *pdata;
>  	struct abx500_bm_data *bat;
>  	struct power_supply btemp_psy;
>  	struct ab8500_btemp_events events;
> @@ -982,7 +982,7 @@ static int __devinit ab8500_btemp_probe(struct platform_device *pdev)
>  	di->gpadc = ab8500_gpadc_get("ab8500-gpadc.0");
>  
>  	/* get btemp specific platform data */
> -	di->pdata = plat_data->btemp;
> +	di->pdata = plat_data->bmdev_pdata;
>  	if (!di->pdata) {
>  		dev_err(di->dev, "no btemp platform data supplied\n");
>  		ret = -EINVAL;
> diff --git a/drivers/power/ab8500_charger.c b/drivers/power/ab8500_charger.c
> index d4f0c98..5ff0d83 100644
> --- a/drivers/power/ab8500_charger.c
> +++ b/drivers/power/ab8500_charger.c
> @@ -220,7 +220,7 @@ struct ab8500_charger {
>  	bool autopower;
>  	struct ab8500 *parent;
>  	struct ab8500_gpadc *gpadc;
> -	struct abx500_charger_platform_data *pdata;
> +	struct abx500_bmdevs_plat_data *pdata;
>  	struct abx500_bm_data *bat;
>  	struct ab8500_charger_event_flags flags;
>  	struct ab8500_charger_usb_state usb_state;
> @@ -2555,7 +2555,7 @@ static int __devinit ab8500_charger_probe(struct platform_device *pdev)
>  	spin_lock_init(&di->usb_state.usb_lock);
>  
>  	/* get charger specific platform data */
> -	di->pdata = plat_data->charger;
> +	di->pdata = plat_data->bmdev_pdata;
>  	if (!di->pdata) {
>  		dev_err(di->dev, "no charger platform data supplied\n");
>  		ret = -EINVAL;
> diff --git a/drivers/power/ab8500_fg.c b/drivers/power/ab8500_fg.c
> index bf02225..96741b8 100644
> --- a/drivers/power/ab8500_fg.c
> +++ b/drivers/power/ab8500_fg.c
> @@ -22,15 +22,14 @@
>  #include <linux/platform_device.h>
>  #include <linux/power_supply.h>
>  #include <linux/kobject.h>
> -#include <linux/mfd/abx500/ab8500.h>
> -#include <linux/mfd/abx500.h>
>  #include <linux/slab.h>
> -#include <linux/mfd/abx500/ab8500-bm.h>
>  #include <linux/delay.h>
> -#include <linux/mfd/abx500/ab8500-gpadc.h>
> -#include <linux/mfd/abx500.h>
>  #include <linux/time.h>
>  #include <linux/completion.h>
> +#include <linux/mfd/abx500.h>
> +#include <linux/mfd/abx500/ab8500.h>
> +#include <linux/mfd/abx500/ab8500-bm.h>
> +#include <linux/mfd/abx500/ab8500-gpadc.h>
>  
>  #define MILLI_TO_MICRO			1000
>  #define FG_LSB_IN_MA			1627
> @@ -212,7 +211,7 @@ struct ab8500_fg {
>  	struct ab8500_fg_avg_cap avg_cap;
>  	struct ab8500 *parent;
>  	struct ab8500_gpadc *gpadc;
> -	struct abx500_fg_platform_data *pdata;
> +	struct abx500_bmdevs_plat_data *pdata;
>  	struct abx500_bm_data *bat;
>  	struct power_supply fg_psy;
>  	struct workqueue_struct *fg_wq;
> @@ -544,14 +543,14 @@ cc_err:
>  		ret = abx500_set_register_interruptible(di->dev,
>  			AB8500_GAS_GAUGE, AB8500_GASG_CC_NCOV_ACCU,
>  			SEC_TO_SAMPLE(10));
> -		if (ret)
> +		if (ret < 0)

I don't 'think' this change is required. abx500_set_register_interruptible
will only return !0 on error.

>  			goto fail;
>  
>  		/* Start the CC */
>  		ret = abx500_set_register_interruptible(di->dev, AB8500_RTC,
>  			AB8500_RTC_CC_CONF_REG,
>  			(CC_DEEP_SLEEP_ENA | CC_PWR_UP_ENA));
> -		if (ret)
> +		if (ret < 0)
>  			goto fail;
>  	} else {
>  		di->turn_off_fg = false;
> @@ -2429,7 +2428,6 @@ static int __devexit ab8500_fg_remove(struct platform_device *pdev)
>  	flush_scheduled_work();
>  	power_supply_unregister(&di->fg_psy);
>  	platform_set_drvdata(pdev, NULL);
> -	kfree(di);
>  	return ret;
>  }
>  
> @@ -2446,18 +2444,47 @@ static int __devinit ab8500_fg_probe(struct platform_device *pdev)
>  {
>  	int i, irq;
>  	int ret = 0;
> -	struct abx500_bm_plat_data *plat_data = pdev->dev.platform_data;
> +	struct abx500_bm_plat_data *plat_data
> +				= pdev->dev.platform_data;
> +	struct device_node *np	= pdev->dev.of_node;
>  	struct ab8500_fg *di;
>  
> +	di = devm_kzalloc(&pdev->dev, sizeof(*di), GFP_KERNEL);
> +	if (!di) {
> +		dev_err(&pdev->dev, "%s no mem for ab8500_fg\n", __func__);
> +		return -ENOMEM;
> +	}
> +	if (np) {
> +		if (!plat_data) {

Change these around.

if (!plat_data) {
        if (np) {
                <snip>
        } else {
                <ERROR>
        }
}

> +			plat_data =
> +			devm_kzalloc(&pdev->dev, sizeof(*plat_data),
> +					GFP_KERNEL);
> +			if (!plat_data) {
> +				dev_err(&pdev->dev,
> +					"%s no mem for plat_data\n", __func__);
> +				return -ENOMEM;
> +			}
> +			plat_data->bmdev_pdata = devm_kzalloc(&pdev->dev,
> +				sizeof(*plat_data->bmdev_pdata), GFP_KERNEL);
> +			if (!plat_data->bmdev_pdata) {
> +				dev_err(&pdev->dev,
> +					"%s no mem for pdata->fg\n",
> +					__func__);
> +				return -ENOMEM;
> +			}
> +		}
> +		ret = bmdevs_of_probe(&pdev->dev, np, plat_data);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "failed to get platform data\n");
> +			return ret;
> +		}
> +	}

<remove>

>  	if (!plat_data) {
> -		dev_err(&pdev->dev, "No platform data\n");
> +		dev_err(&pdev->dev,
> +			"%s no fg platform data found\n", __func__);
>  		return -EINVAL;
>  	}
</remove>

> -	di = kzalloc(sizeof(*di), GFP_KERNEL);
> -	if (!di)
> -		return -ENOMEM;
> -
>  	mutex_init(&di->cc_lock);
>  
>  	/* get parent data */
> @@ -2466,19 +2493,17 @@ static int __devinit ab8500_fg_probe(struct platform_device *pdev)
>  	di->gpadc = ab8500_gpadc_get("ab8500-gpadc.0");
>  
>  	/* get fg specific platform data */
> -	di->pdata = plat_data->fg;
> +	di->pdata = plat_data->bmdev_pdata;
>  	if (!di->pdata) {
>  		dev_err(di->dev, "no fg platform data supplied\n");
> -		ret = -EINVAL;
> -		goto free_device_info;
> +		return -EINVAL;
>  	}
>  
>  	/* get battery specific platform data */
>  	di->bat = plat_data->battery;
>  	if (!di->bat) {
>  		dev_err(di->dev, "no battery platform data supplied\n");
> -		ret = -EINVAL;
> -		goto free_device_info;
> +		return -EINVAL;
>  	}
>  
>  	di->fg_psy.name = "ab8500_fg";
> @@ -2506,7 +2531,7 @@ static int __devinit ab8500_fg_probe(struct platform_device *pdev)
>  	di->fg_wq = create_singlethread_workqueue("ab8500_fg_wq");
>  	if (di->fg_wq == NULL) {
>  		dev_err(di->dev, "failed to create work queue\n");
> -		goto free_device_info;
> +		return -ENOMEM;
>  	}
>  
>  	/* Init work for running the fg algorithm instantly */
> @@ -2605,12 +2630,14 @@ free_irq:
>  	}
>  free_inst_curr_wq:
>  	destroy_workqueue(di->fg_wq);
> -free_device_info:
> -	kfree(di);
> -
>  	return ret;
>  }
>  
> +static const struct of_device_id ab8500_fg_match[] = {
> +	{.compatible = "stericsson,ab8500-fg",},

<nit>

Spaces:

{ .compatible = "stericsson,ab8500-fg", },

</nit>

> +	{},
> +};
> +
>  static struct platform_driver ab8500_fg_driver = {
>  	.probe = ab8500_fg_probe,
>  	.remove = __devexit_p(ab8500_fg_remove),
> @@ -2619,6 +2646,7 @@ static struct platform_driver ab8500_fg_driver = {
>  	.driver = {
>  		.name = "ab8500-fg",
>  		.owner = THIS_MODULE,
> +		.of_match_table = ab8500_fg_match,
>  	},
>  };
>  
> diff --git a/drivers/power/abx500_chargalg.c b/drivers/power/abx500_chargalg.c
> index 804b88c..ba548e4 100644
> --- a/drivers/power/abx500_chargalg.c
> +++ b/drivers/power/abx500_chargalg.c
> @@ -231,7 +231,7 @@ struct abx500_chargalg {
>  	struct abx500_chargalg_charger_info chg_info;
>  	struct abx500_chargalg_battery_data batt_data;
>  	struct abx500_chargalg_suspension_status susp_status;
> -	struct abx500_chargalg_platform_data *pdata;
> +	struct abx500_bmdevs_plat_data *pdata;
>  	struct abx500_bm_data *bat;
>  	struct power_supply chargalg_psy;
>  	struct ux500_charger *ac_chg;
> @@ -1814,7 +1814,7 @@ static int __devinit abx500_chargalg_probe(struct platform_device *pdev)
>  	di->dev = &pdev->dev;
>  
>  	plat_data = pdev->dev.platform_data;
> -	di->pdata = plat_data->chargalg;
> +	di->pdata = plat_data->bmdev_pdata;
>  	di->bat = plat_data->battery;
>  
>  	/* chargalg supply */
> diff --git a/include/linux/mfd/abx500.h b/include/linux/mfd/abx500.h
> index 1318ca6..286f8ac 100644
> --- a/include/linux/mfd/abx500.h
> +++ b/include/linux/mfd/abx500.h
> @@ -382,39 +382,30 @@ struct abx500_bm_data {
>  	int gnd_lift_resistance;
>  	const struct abx500_maxim_parameters *maxi;
>  	const struct abx500_bm_capacity_levels *cap_levels;
> -	const struct abx500_battery_type *bat_type;
> +	struct abx500_battery_type *bat_type;
>  	const struct abx500_bm_charger_parameters *chg_params;
>  	const struct abx500_fg_parameters *fg_params;
>  };
>  
> -struct abx500_chargalg_platform_data {
> -	char **supplied_to;
> -	size_t num_supplicants;
> +struct abx500_bmdevs_plat_data {
> +	char	**supplied_to;
> +	size_t	num_supplicants;
> +	bool	autopower_cfg;
>  };
>  
> -struct abx500_charger_platform_data {
> -	char **supplied_to;
> -	size_t num_supplicants;
> -	bool autopower_cfg;
> -};
> -
> -struct abx500_btemp_platform_data {
> -	char **supplied_to;
> -	size_t num_supplicants;
> +struct abx500_bm_plat_data {
> +	struct abx500_bm_data *battery;
> +	struct abx500_bmdevs_plat_data *bmdev_pdata;
>  };
>  
> -struct abx500_fg_platform_data {
> -	char **supplied_to;
> -	size_t num_supplicants;
> +enum {
> +	NTC_EXTERNAL = 0,
> +	NTC_INTERNAL,
>  };
>  
> -struct abx500_bm_plat_data {
> -	struct abx500_bm_data *battery;
> -	struct abx500_charger_platform_data *charger;
> -	struct abx500_btemp_platform_data *btemp;
> -	struct abx500_fg_platform_data *fg;
> -	struct abx500_chargalg_platform_data *chargalg;
> -};
> +int bmdevs_of_probe(struct device *dev,
> +		struct device_node *np,
> +		struct abx500_bm_plat_data *pdata);
>  
>  int abx500_set_register_interruptible(struct device *dev, u8 bank, u8 reg,
>  	u8 value);
> diff --git a/include/linux/mfd/abx500/ab8500-bm.h b/include/linux/mfd/abx500/ab8500-bm.h
> index 44310c9..d15b7f1 100644
> --- a/include/linux/mfd/abx500/ab8500-bm.h
> +++ b/include/linux/mfd/abx500/ab8500-bm.h
> @@ -422,6 +422,13 @@ struct ab8500_chargalg_platform_data {
>  struct ab8500_btemp;
>  struct ab8500_gpadc;
>  struct ab8500_fg;
> +
> +extern struct abx500_bm_data ab8500_bm_data;
> +extern struct abx500_battery_type bat_type_ext_thermistor[];
> +extern struct batres_vs_temp temp_to_batres_tbl_ext_thermistor[];
> +extern struct batres_vs_temp temp_to_batres_tbl_9100[];
> +extern struct batres_vs_temp temp_to_batres_tbl_thermistor[];
> +
>  #ifdef CONFIG_AB8500_BM
>  void ab8500_fg_reinit(void);
>  void ab8500_charger_usb_state_changed(u8 bm_usb_state, u16 mA);
> -- 
> 1.7.9.5
>

Rajanikanth H.V Oct. 1, 2012, 9:59 a.m. UTC | #2

did you have a look at arnd and anton comments regarding
'supplied-to' and boolean property

On Monday 01 October 2012 03:19 PM, Lee Jones wrote:
> On Mon, 01 Oct 2012, Rajanikanth H.V wrote:
> 
>> From: "Rajanikanth H.V" <rajanikanth.hv@stericsson.com>
>>
>> - This patch adds device tree support for fuelguage driver
>> - optimize bm devices platform_data usage and of_probe(...)
>>   Note: of_probe() routine for battery managed devices is made
>>   common across all bm drivers.
>>
>> Signed-off-by: Rajanikanth H.V <rajanikanth.hv@stericsson.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/ab8500.txt   |    8 +-
>>  .../devicetree/bindings/power_supply/ab8500/fg.txt |   86 +++
>>  arch/arm/boot/dts/dbx5x0.dtsi                      |   22 +-
>>  drivers/mfd/ab8500-core.c                          |    1 +
>>  drivers/power/Makefile                             |    2 +-
>>  drivers/power/ab8500_bmdata.c                      |  549 ++++++++++++++++++++
>>  drivers/power/ab8500_btemp.c                       |    4 +-
>>  drivers/power/ab8500_charger.c                     |    4 +-
>>  drivers/power/ab8500_fg.c                          |   76 ++-
>>  drivers/power/abx500_chargalg.c                    |    4 +-
>>  include/linux/mfd/abx500.h                         |   37 +-
>>  include/linux/mfd/abx500/ab8500-bm.h               |    7 +
>>  12 files changed, 744 insertions(+), 56 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/power_supply/ab8500/fg.txt
>>  create mode 100644 drivers/power/ab8500_bmdata.c
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/ab8500.txt b/Documentation/devicetree/bindings/mfd/ab8500.txt
>> index ce83c8d..762dc11 100644
>> --- a/Documentation/devicetree/bindings/mfd/ab8500.txt
>> +++ b/Documentation/devicetree/bindings/mfd/ab8500.txt
>> @@ -24,7 +24,13 @@ ab8500-bm                :                      :              : Battery Manager
>>  ab8500-btemp             :                      :              : Battery Temperature
>>  ab8500-charger           :                      :              : Battery Charger
>>  ab8500-codec             :                      :              : Audio Codec
>> -ab8500-fg                :                      :              : Fuel Gauge
>> +ab8500-fg                :                   : vddadc       : Fuel Gauge
>> +                      : NCONV_ACCU           :              : Accumulate N Sample Conversion
>> +                      : BATT_OVV             :              : Battery Over Voltage
>> +                      : LOW_BAT_F            :              : LOW threshold battery voltage
>> +                      : CC_INT_CALIB         :              : Counter Counter Internal Calibration
> 
> I think you mean: Coulomb Counter.
> 
>> +                      : CCEOC                :              : Coulomb Counter End of Conversion
>> +                      :                      :              :
> 
> Random empty entry.
> 
>>  ab8500-gpadc             : HW_CONV_END          : vddadc       : Analogue to Digital Converter
>>                             SW_CONV_END          :              :
>>  ab8500-gpio              :                      :              : GPIO Controller
>> diff --git a/Documentation/devicetree/bindings/power_supply/ab8500/fg.txt b/Documentation/devicetree/bindings/power_supply/ab8500/fg.txt
>> new file mode 100644
>> index 0000000..caa33b0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power_supply/ab8500/fg.txt
>> @@ -0,0 +1,86 @@
>> +=== AB8500 Fuel Gauge Driver ===
>> +
>> +AB8500 is a mixed signal multimedia and power management
>> +device comprising: power and energy-management-module,
>> +wall-charger, usb-charger, audio codec, general purpose adc,
>> +tvout, clock management and sim card interface.
>> +
>> +Fuel-guage support is part of energy-management-module, the other
> 
> Spelling.
> 
>> +components of this module are:
>> +main-charger, usb-combo-charger and Battery temperature monitoring.
>> +
>> +The properties below describes the node for fuel guage driver.
> 
> Spelling.
> 
>> +
>> +Required Properties:
>> +- compatible = "stericsson,ab8500-fg"
>> +- interface-name:
>> +     Name of the controller/driver which is part of energy-management-module
>> +- supplied-to:
> 
> Still not sure about this property, or your justification for use.
> 
>> +     This property shall have dependent nodes which represent other
>> +     energy-management-module.
> 
> Plural?
> 
>> +     This is a logical binding w.r.t power supply events
> 
> Proper English please, no slang.
> 
>> +     across energy-management-module drivers where-in, the
> 
> Ill placed comma?
> 
>> +     runtime battery properties are shared along with uevent
>> +     notification.
> 
> Plural?
> 
>> +     ref: di->fg.external_power_changed =
>> +             ab8500_fg_external_power_changed;
>> +             ab8500_fg.c
>> +
>> +     Need for this property:
>> +             energy-management-module driver updates power-supply properties
>> +             which are subset of events listed in 'enum power_supply_property',
>> +             ref: power_supply.h file
>> +             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
>> +             di->fg_psy.external_power_changed
>> +
>> +     example:
>> +     ab8500-fg {
>> +             /* dependent energy management modules */
>> +             supplied-to  = <&ab8500_chargalg &ab8500_usb>;
>> +     };
>> +
>> +     ab8500_battery_info: ab8500_bat_type {
>> +             battery-type = <2>;
>> +             thermistor-on-batctrl = <1>;
> 
> You have this as a bool here, and ...
>> +     };
>> +
>> +Other dependent node for fuel-gauge is:
>> +     ab8500_battery_info: ab8500_bat_type {
>> +     };
>> +     This node will provide information on 'thermistor interface' and
>> +     'battery technology type' used.
>> +
>> +Properties of this node are:
>> +thermistor-on-batctrl:
>> +     A boolean value indicating thermistor interface to battery
>> +
>> +     Note:
>> +     'btemp' and 'batctrl' are the pins interfaced for battery temperature
>> +     measurement, 'btemp' signal is used when NTC(negative temperature
>> +     coefficient) resister is interfaced external to battery whereas
>> +     'batctrl' pin is used when NTC resister is internal to battery.
>> +
>> +     e.g:
>> +     ab8500_battery_info: ab8500_bat_type {
>> +             thermistor-on-batctrl;
> 
> ... a standard property here. I suggest you drop the bool value.
> 
>> +     };
>> +     indiactes: NTC resister is internal to battery, 'batctrl' is used
>> +             for thermal measurement.
>> +
>> +     The absence of property 'thermal-on-batctrl' indicates
>> +     NTC resister is external to battery and  'btemp' signal is used
>> +     for thermal measurement.
>> +
>> +battery-type:
>> +     This shall be the battery manufacturing technology type,
>> +     allowed types are:
>> +             "UNKNOWN" "NiMH" "LION" "LIPO" "LiFe" "NiCd" "LiMn"
>> +     e.g:
>> +     ab8500_battery_info: ab8500_bat_type {
>> +             battery-name = "LION";
>> +     }
>> +
>> diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
>> index 748ba7a..bd22c56 100644
>> --- a/arch/arm/boot/dts/dbx5x0.dtsi
>> +++ b/arch/arm/boot/dts/dbx5x0.dtsi
>> @@ -352,8 +352,28 @@
>>                                       vddadc-supply = <&ab8500_ldo_tvout_reg>;
>>                               };
>>
>> -                             ab8500-usb {
>> +                             ab8500_battery_info: ab8500_bat_type {
>> +                                     battery-name = "LION";
> 
> All new properties have to be documented.
> 
> Vendor specific properties should be prepended with the vendor name, so
> either write a generic binding document for all to use or prefix with
> 'stericsson,".
> 
>> +                                     thermistor-on-batctrl;
>> +                             };
>> +
>> +                             ab8500_chargalg: ab8500_chalg {
>> +                                     compatible     = "stericsson,ab8500-chargalg";
>> +                                     interface-name = "ab8500_chargalg";
> 
> Same with all of your new properties (I'll stop mentioning them now).
> 
>> +                                     battery-info   = <&ab8500_battery_info>;
>> +                                     supplied-to    = <&ab8500_fuel_gauge>;
> 
> Weren't you going to reverse this logic to be more inline with how
> the reset of Device Tree works?
> 
>> +                             };
>> +
>> +                             ab8500_fuel_gauge: ab8500_fg {
>> +                                     compatible     = "stericsson,ab8500-fg";
>> +                                     interface-name = "ab8500_fg";
>> +                                     battery-info   = <&ab8500_battery_info>;
>> +                                     supplied-to    = <&ab8500_chargalg &ab8500_usb>;
> 
> As above.
> 
>> +                             };
>> +
>> +                             ab8500_usb: ab8500_usb_if {
> 
> What does 'if' mean?
> 
>>                                       compatible = "stericsson,ab8500-usb";
>> +                                     interface-name = "ab8500_usb";
> 
> Why is this required?
> 
>>                                       interrupts = < 90 0x4
>>                                                      96 0x4
>>                                                      14 0x4
>> diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
>> index 1667c77..6c3d7c2 100644
>> --- a/drivers/mfd/ab8500-core.c
>> +++ b/drivers/mfd/ab8500-core.c
>> @@ -1051,6 +1051,7 @@ static struct mfd_cell __devinitdata ab8500_bm_devs[] = {
>>       },
>>       {
>>               .name = "ab8500-fg",
>> +             .of_compatible = "stericsson,ab8500-fg",
>>               .num_resources = ARRAY_SIZE(ab8500_fg_resources),
>>               .resources = ab8500_fg_resources,
>>       },
>> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
>> index ee58afb..2c58d4e 100644
>> --- a/drivers/power/Makefile
>> +++ b/drivers/power/Makefile
>> @@ -34,7 +34,7 @@ obj-$(CONFIG_BATTERY_S3C_ADC)       += s3c_adc_battery.o
>>  obj-$(CONFIG_CHARGER_PCF50633)       += pcf50633-charger.o
>>  obj-$(CONFIG_BATTERY_JZ4740) += jz4740-battery.o
>>  obj-$(CONFIG_BATTERY_INTEL_MID)      += intel_mid_battery.o
>> -obj-$(CONFIG_AB8500_BM)              += ab8500_charger.o ab8500_btemp.o ab8500_fg.o abx500_chargalg.o
>> +obj-$(CONFIG_AB8500_BM)              += ab8500_bmdata.o ab8500_charger.o ab8500_fg.o ab8500_btemp.o abx500_chargalg.o
>>  obj-$(CONFIG_CHARGER_ISP1704)        += isp1704_charger.o
>>  obj-$(CONFIG_CHARGER_MAX8903)        += max8903_charger.o
>>  obj-$(CONFIG_CHARGER_TWL4030)        += twl4030_charger.o
>> diff --git a/drivers/power/ab8500_bmdata.c b/drivers/power/ab8500_bmdata.c
>> new file mode 100644
>> index 0000000..d0def3b
>> --- /dev/null
>> +++ b/drivers/power/ab8500_bmdata.c
>> @@ -0,0 +1,549 @@
>> +#include <linux/export.h>
>> +#include <linux/power_supply.h>
>> +#include <linux/of.h>
>> +#include <linux/mfd/abx500.h>
>> +#include <linux/mfd/abx500/ab8500.h>
>> +#include <linux/mfd/abx500/ab8500-bm.h>
>> +
>> +/*
>> + * These are the defined batteries that uses a NTC and ID resistor placed
>> + * inside of the battery pack.
>> + * Note that the res_to_temp table must be strictly sorted by falling resistance
>> + * values to work.
>> + */
>> +static struct abx500_res_to_temp temp_tbl_A_thermistor[] = {
>> +     {-5, 53407},
>> +     { 0, 48594},
>> +     { 5, 43804},
>> +     {10, 39188},
>> +     {15, 34870},
>> +     {20, 30933},
>> +     {25, 27422},
>> +     {30, 24347},
>> +     {35, 21694},
>> +     {40, 19431},
>> +     {45, 17517},
>> +     {50, 15908},
>> +     {55, 14561},
>> +     {60, 13437},
>> +     {65, 12500},
>> +};
>> +static struct abx500_res_to_temp temp_tbl_B_thermistor[] = {
>> +     {-5, 165418},
>> +     { 0, 159024},
>> +     { 5, 151921},
>> +     {10, 144300},
>> +     {15, 136424},
>> +     {20, 128565},
>> +     {25, 120978},
>> +     {30, 113875},
>> +     {35, 107397},
>> +     {40, 101629},
>> +     {45,  96592},
>> +     {50,  92253},
>> +     {55,  88569},
>> +     {60,  85461},
>> +     {65,  82869},
>> +};
>> +static struct abx500_v_to_cap cap_tbl_A_thermistor[] = {
>> +     {4171,  100},
>> +     {4114,   95},
>> +     {4009,   83},
>> +     {3947,   74},
>> +     {3907,   67},
>> +     {3863,   59},
>> +     {3830,   56},
>> +     {3813,   53},
>> +     {3791,   46},
>> +     {3771,   33},
>> +     {3754,   25},
>> +     {3735,   20},
>> +     {3717,   17},
>> +     {3681,   13},
>> +     {3664,    8},
>> +     {3651,    6},
>> +     {3635,    5},
>> +     {3560,    3},
>> +     {3408,    1},
>> +     {3247,    0},
>> +};
>> +static struct abx500_v_to_cap cap_tbl_B_thermistor[] = {
>> +     {4161,  100},
>> +     {4124,   98},
>> +     {4044,   90},
>> +     {4003,   85},
>> +     {3966,   80},
>> +     {3933,   75},
>> +     {3888,   67},
>> +     {3849,   60},
>> +     {3813,   55},
>> +     {3787,   47},
>> +     {3772,   30},
>> +     {3751,   25},
>> +     {3718,   20},
>> +     {3681,   16},
>> +     {3660,   14},
>> +     {3589,   10},
>> +     {3546,    7},
>> +     {3495,    4},
>> +     {3404,    2},
>> +     {3250,    0},
>> +};
>> +
>> +static struct abx500_v_to_cap cap_tbl[] = {
>> +     {4186,  100},
>> +     {4163,   99},
>> +     {4114,   95},
>> +     {4068,   90},
>> +     {3990,   80},
>> +     {3926,   70},
>> +     {3898,   65},
>> +     {3866,   60},
>> +     {3833,   55},
>> +     {3812,   50},
>> +     {3787,   40},
>> +     {3768,   30},
>> +     {3747,   25},
>> +     {3730,   20},
>> +     {3705,   15},
>> +     {3699,   14},
>> +     {3684,   12},
>> +     {3672,    9},
>> +     {3657,    7},
>> +     {3638,    6},
>> +     {3556,    4},
>> +     {3424,    2},
>> +     {3317,    1},
>> +     {3094,    0},
>> +};
>> +
>> +/*
>> + * Note that the res_to_temp table must be strictly sorted by falling
>> + * resistance values to work.
>> + */
>> +static struct abx500_res_to_temp temp_tbl[] = {
>> +     {-5, 214834},
>> +     { 0, 162943},
>> +     { 5, 124820},
>> +     {10,  96520},
>> +     {15,  75306},
>> +     {20,  59254},
>> +     {25,  47000},
>> +     {30,  37566},
>> +     {35,  30245},
>> +     {40,  24520},
>> +     {45,  20010},
>> +     {50,  16432},
>> +     {55,  13576},
>> +     {60,  11280},
>> +     {65,   9425},
>> +};
>> +
>> +/*
>> + * Note that the batres_vs_temp table must be strictly sorted by falling
>> + * temperature values to work.
>> + */
>> +struct batres_vs_temp temp_to_batres_tbl_thermistor[] = {
>> +     { 40, 120},
>> +     { 30, 135},
>> +     { 20, 165},
>> +     { 10, 230},
>> +     { 00, 325},
>> +     {-10, 445},
>> +     {-20, 595},
>> +};
>> +
>> +/*
>> + * Note that the batres_vs_temp table must be strictly sorted by falling
>> + * temperature values to work.
>> + */
>> +struct batres_vs_temp temp_to_batres_tbl_ext_thermistor[] = {
>> +     { 60, 300},
>> +     { 30, 300},
>> +     { 20, 300},
>> +     { 10, 300},
>> +     { 00, 300},
>> +     {-10, 300},
>> +     {-20, 300},
>> +};
>> +
>> +/* battery resistance table for LI ION 9100 battery */
>> +struct batres_vs_temp temp_to_batres_tbl_9100[] = {
>> +     { 60, 180},
>> +     { 30, 180},
>> +     { 20, 180},
>> +     { 10, 180},
>> +     { 00, 180},
>> +     {-10, 180},
>> +     {-20, 180},
>> +};
>> +
>> +struct abx500_battery_type bat_type_thermistor[] = {
>> +[BATTERY_UNKNOWN] = {
>> +     /* First element always represent the UNKNOWN battery */
>> +     .name = POWER_SUPPLY_TECHNOLOGY_UNKNOWN,
>> +     .resis_high = 0,
>> +     .resis_low = 0,
>> +     .battery_resistance = 300,
>> +     .charge_full_design = 612,
>> +     .nominal_voltage = 3700,
>> +     .termination_vol = 4050,
>> +     .termination_curr = 200,
>> +     .recharge_vol = 3990,
>> +     .normal_cur_lvl = 400,
>> +     .normal_vol_lvl = 4100,
>> +     .maint_a_cur_lvl = 400,
>> +     .maint_a_vol_lvl = 4050,
>> +     .maint_a_chg_timer_h = 60,
>> +     .maint_b_cur_lvl = 400,
>> +     .maint_b_vol_lvl = 4000,
>> +     .maint_b_chg_timer_h = 200,
>> +     .low_high_cur_lvl = 300,
>> +     .low_high_vol_lvl = 4000,
>> +     .n_temp_tbl_elements = ARRAY_SIZE(temp_tbl),
>> +     .r_to_t_tbl = temp_tbl,
>> +     .n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl),
>> +     .v_to_cap_tbl = cap_tbl,
>> +     .n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
>> +     .batres_tbl = temp_to_batres_tbl_thermistor,
>> +},
>> +{
>> +     .name = POWER_SUPPLY_TECHNOLOGY_LIPO,
>> +     .resis_high = 53407,
>> +     .resis_low = 12500,
>> +     .battery_resistance = 300,
>> +     .charge_full_design = 900,
>> +     .nominal_voltage = 3600,
>> +     .termination_vol = 4150,
>> +     .termination_curr = 80,
>> +     .recharge_vol = 4130,
>> +     .normal_cur_lvl = 700,
>> +     .normal_vol_lvl = 4200,
>> +     .maint_a_cur_lvl = 600,
>> +     .maint_a_vol_lvl = 4150,
>> +     .maint_a_chg_timer_h = 60,
>> +     .maint_b_cur_lvl = 600,
>> +     .maint_b_vol_lvl = 4100,
>> +     .maint_b_chg_timer_h = 200,
>> +     .low_high_cur_lvl = 300,
>> +     .low_high_vol_lvl = 4000,
>> +     .n_temp_tbl_elements = ARRAY_SIZE(temp_tbl_A_thermistor),
>> +     .r_to_t_tbl = temp_tbl_A_thermistor,
>> +     .n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl_A_thermistor),
>> +     .v_to_cap_tbl = cap_tbl_A_thermistor,
>> +     .n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
>> +     .batres_tbl = temp_to_batres_tbl_thermistor,
>> +
>> +},
>> +{
>> +     .name = POWER_SUPPLY_TECHNOLOGY_LIPO,
>> +     .resis_high = 165418,
>> +     .resis_low = 82869,
>> +     .battery_resistance = 300,
>> +     .charge_full_design = 900,
>> +     .nominal_voltage = 3600,
>> +     .termination_vol = 4150,
>> +     .termination_curr = 80,
>> +     .recharge_vol = 4130,
>> +     .normal_cur_lvl = 700,
>> +     .normal_vol_lvl = 4200,
>> +     .maint_a_cur_lvl = 600,
>> +     .maint_a_vol_lvl = 4150,
>> +     .maint_a_chg_timer_h = 60,
>> +     .maint_b_cur_lvl = 600,
>> +     .maint_b_vol_lvl = 4100,
>> +     .maint_b_chg_timer_h = 200,
>> +     .low_high_cur_lvl = 300,
>> +     .low_high_vol_lvl = 4000,
>> +     .n_temp_tbl_elements = ARRAY_SIZE(temp_tbl_B_thermistor),
>> +     .r_to_t_tbl = temp_tbl_B_thermistor,
>> +     .n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl_B_thermistor),
>> +     .v_to_cap_tbl = cap_tbl_B_thermistor,
>> +     .n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
>> +     .batres_tbl = temp_to_batres_tbl_thermistor,
>> +},
>> +};
>> +
>> +struct abx500_battery_type bat_type_ext_thermistor[] = {
>> +[BATTERY_UNKNOWN] = {
>> +     /* First element always represent the UNKNOWN battery */
>> +     .name = POWER_SUPPLY_TECHNOLOGY_UNKNOWN,
>> +     .resis_high = 0,
>> +     .resis_low = 0,
>> +     .battery_resistance = 300,
>> +     .charge_full_design = 612,
>> +     .nominal_voltage = 3700,
>> +     .termination_vol = 4050,
>> +     .termination_curr = 200,
>> +     .recharge_vol = 3990,
>> +     .normal_cur_lvl = 400,
>> +     .normal_vol_lvl = 4100,
>> +     .maint_a_cur_lvl = 400,
>> +     .maint_a_vol_lvl = 4050,
>> +     .maint_a_chg_timer_h = 60,
>> +     .maint_b_cur_lvl = 400,
>> +     .maint_b_vol_lvl = 4000,
>> +     .maint_b_chg_timer_h = 200,
>> +     .low_high_cur_lvl = 300,
>> +     .low_high_vol_lvl = 4000,
>> +     .n_temp_tbl_elements = ARRAY_SIZE(temp_tbl),
>> +     .r_to_t_tbl = temp_tbl,
>> +     .n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl),
>> +     .v_to_cap_tbl = cap_tbl,
>> +     .n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
>> +     .batres_tbl = temp_to_batres_tbl_thermistor,
>> +},
>> +/*
>> + * These are the batteries that doesn't have an internal NTC resistor to measure
>> + * its temperature. The temperature in this case is measure with a NTC placed
>> + * near the battery but on the PCB.
>> + */
>> +{
>> +     .name = POWER_SUPPLY_TECHNOLOGY_LIPO,
>> +     .resis_high = 76000,
>> +     .resis_low = 53000,
>> +     .battery_resistance = 300,
>> +     .charge_full_design = 900,
>> +     .nominal_voltage = 3700,
>> +     .termination_vol = 4150,
>> +     .termination_curr = 100,
>> +     .recharge_vol = 4130,
>> +     .normal_cur_lvl = 700,
>> +     .normal_vol_lvl = 4200,
>> +     .maint_a_cur_lvl = 600,
>> +     .maint_a_vol_lvl = 4150,
>> +     .maint_a_chg_timer_h = 60,
>> +     .maint_b_cur_lvl = 600,
>> +     .maint_b_vol_lvl = 4100,
>> +     .maint_b_chg_timer_h = 200,
>> +     .low_high_cur_lvl = 300,
>> +     .low_high_vol_lvl = 4000,
>> +     .n_temp_tbl_elements = ARRAY_SIZE(temp_tbl),
>> +     .r_to_t_tbl = temp_tbl,
>> +     .n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl),
>> +     .v_to_cap_tbl = cap_tbl,
>> +     .n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
>> +     .batres_tbl = temp_to_batres_tbl_thermistor,
>> +},
>> +{
>> +     .name = POWER_SUPPLY_TECHNOLOGY_LION,
>> +     .resis_high = 30000,
>> +     .resis_low = 10000,
>> +     .battery_resistance = 300,
>> +     .charge_full_design = 950,
>> +     .nominal_voltage = 3700,
>> +     .termination_vol = 4150,
>> +     .termination_curr = 100,
>> +     .recharge_vol = 4130,
>> +     .normal_cur_lvl = 700,
>> +     .normal_vol_lvl = 4200,
>> +     .maint_a_cur_lvl = 600,
>> +     .maint_a_vol_lvl = 4150,
>> +     .maint_a_chg_timer_h = 60,
>> +     .maint_b_cur_lvl = 600,
>> +     .maint_b_vol_lvl = 4100,
>> +     .maint_b_chg_timer_h = 200,
>> +     .low_high_cur_lvl = 300,
>> +     .low_high_vol_lvl = 4000,
>> +     .n_temp_tbl_elements = ARRAY_SIZE(temp_tbl),
>> +     .r_to_t_tbl = temp_tbl,
>> +     .n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl),
>> +     .v_to_cap_tbl = cap_tbl,
>> +     .n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
>> +     .batres_tbl = temp_to_batres_tbl_thermistor,
>> +},
>> +{
>> +     .name = POWER_SUPPLY_TECHNOLOGY_LION,
>> +     .resis_high = 95000,
>> +     .resis_low = 76001,
>> +     .battery_resistance = 300,
>> +     .charge_full_design = 950,
>> +     .nominal_voltage = 3700,
>> +     .termination_vol = 4150,
>> +     .termination_curr = 100,
>> +     .recharge_vol = 4130,
>> +     .normal_cur_lvl = 700,
>> +     .normal_vol_lvl = 4200,
>> +     .maint_a_cur_lvl = 600,
>> +     .maint_a_vol_lvl = 4150,
>> +     .maint_a_chg_timer_h = 60,
>> +     .maint_b_cur_lvl = 600,
>> +     .maint_b_vol_lvl = 4100,
>> +     .maint_b_chg_timer_h = 200,
>> +     .low_high_cur_lvl = 300,
>> +     .low_high_vol_lvl = 4000,
>> +     .n_temp_tbl_elements = ARRAY_SIZE(temp_tbl),
>> +     .r_to_t_tbl = temp_tbl,
>> +     .n_v_cap_tbl_elements = ARRAY_SIZE(cap_tbl),
>> +     .v_to_cap_tbl = cap_tbl,
>> +     .n_batres_tbl_elements = ARRAY_SIZE(temp_to_batres_tbl_thermistor),
>> +     .batres_tbl = temp_to_batres_tbl_thermistor,
>> +},
>> +};
>> +
>> +static const struct abx500_bm_capacity_levels cap_levels = {
>> +     .critical       = 2,
>> +     .low            = 10,
>> +     .normal         = 70,
>> +     .high           = 95,
>> +     .full           = 100,
>> +};
>> +
>> +static const struct abx500_fg_parameters fg = {
>> +     .recovery_sleep_timer = 10,
>> +     .recovery_total_time = 100,
>> +     .init_timer = 1,
>> +     .init_discard_time = 5,
>> +     .init_total_time = 40,
>> +     .high_curr_time = 60,
>> +     .accu_charging = 30,
>> +     .accu_high_curr = 30,
>> +     .high_curr_threshold = 50,
>> +     .lowbat_threshold = 3100,
>> +     .battok_falling_th_sel0 = 2860,
>> +     .battok_raising_th_sel1 = 2860,
>> +     .user_cap_limit = 15,
>> +     .maint_thres = 97,
>> +};
>> +
>> +static const struct abx500_maxim_parameters maxi_params = {
>> +     .ena_maxi = true,
>> +     .chg_curr = 910,
>> +     .wait_cycles = 10,
>> +     .charger_curr_step = 100,
>> +};
>> +
>> +static const struct abx500_bm_charger_parameters chg = {
>> +     .usb_volt_max           = 5500,
>> +     .usb_curr_max           = 1500,
>> +     .ac_volt_max            = 7500,
>> +     .ac_curr_max            = 1500,
>> +};
>> +
>> +struct abx500_bm_data ab8500_bm_data = {
>> +     .temp_under             = 3,
>> +     .temp_low               = 8,
>> +     .temp_high              = 43,
>> +     .temp_over              = 48,
>> +     .main_safety_tmr_h      = 4,
>> +     .temp_interval_chg      = 20,
>> +     .temp_interval_nochg    = 120,
>> +     .usb_safety_tmr_h       = 4,
>> +     .bkup_bat_v             = BUP_VCH_SEL_2P6V,
>> +     .bkup_bat_i             = BUP_ICH_SEL_150UA,
>> +     .no_maintenance         = false,
>> +     .adc_therm              = ABx500_ADC_THERM_BATCTRL,
>> +     .chg_unknown_bat        = false,
>> +     .enable_overshoot       = false,
>> +     .fg_res                 = 100,
>> +     .cap_levels             = &cap_levels,
>> +     .bat_type               = bat_type_thermistor,
>> +     .n_btypes               = 3,
>> +     .batt_id                = 0,
>> +     .interval_charging      = 5,
>> +     .interval_not_charging  = 120,
>> +     .temp_hysteresis        = 3,
>> +     .gnd_lift_resistance    = 34,
>> +     .maxi                   = &maxi_params,
>> +     .chg_params             = &chg,
>> +     .fg_params              = &fg,
>> +};
>> +
>> +int __devinit
>> +bmdevs_of_probe(struct device *dev,
>> +             struct device_node *np,
>> +             struct abx500_bm_plat_data *pdata)
>> +{
>> +     int     i, ret = 0, thermistor = NTC_INTERNAL;
>> +     const   __be32 *ph;
>> +     const   char *bat_tech;
>> +     struct  abx500_bm_data           *bat;
>> +     struct  abx500_battery_type      *btype;
>> +     struct  device_node              *np_bat_supply;
>> +     struct  abx500_bmdevs_plat_data  *plat_data = pdata->bmdev_pdata;
> 
> <nit>
> 
> This spacing is uncharacteristic of Linux drivers.
> 
> Usually, struct declarations come first.
> 
> </nit>
> 
>> +     /* get phandle to 'supplied-to' node */
> 
> I thought you were going to reverse this?
> 
>> +     ph = of_get_property(np, "supplied-to", &plat_data->num_supplicants);
>> +     if (ph == NULL) {
> 
> if (!ph) {
> 
>> +             dev_err(dev, "no supplied_to property specified\n");
>> +             return -EINVAL;
>> +     }
>> +     plat_data->num_supplicants /= sizeof(int);
>> +     plat_data->supplied_to =
>> +             devm_kzalloc(dev, plat_data->num_supplicants *
>> +                     sizeof(const char *), GFP_KERNEL);
>> +     if (plat_data->supplied_to == NULL) {
>> +             dev_err(dev, "%s no mem for supplied-to\n", __func__);
>> +             return -ENOMEM;
>> +     }
>> +     for (i = 0; i < plat_data->num_supplicants; ++i) {
>> +             np_bat_supply = of_find_node_by_phandle(be32_to_cpup(ph) + i);
> 
> Use: of_parse_phandle(np, "supplied-to", i) instead.
> 
>> +             if (np_bat_supply == NULL) {
> 
> if (!np_bat_supply) {
> 
>> +                     dev_err(dev, "invalid supplied_to property\n");
>> +                     return -EINVAL;
>> +             }
>> +             ret = of_property_read_string(np_bat_supply, "interface-name",
>> +                             (const char **)(plat_data->supplied_to + i));
>> +             if (ret < 0) {
>> +                     of_node_put(np_bat_supply);
>> +                     dev_err(dev, "supply/interface name not found\n");
>> +                     return ret;
>> +             }
>> +             dev_dbg(dev, "%s power supply interface_name:%s\n",
>> +                     __func__, *(plat_data->supplied_to + i));
>> +     }
> 
> <remove>
> 
>> +     /* get phandle to 'battery-info' node */
>> +     ph = of_get_property(np, "battery-info", NULL);
>> +     if (ph == NULL) {
>> +             dev_err(dev, "missing property battery-info\n");
>> +             return -EINVAL;
>> +     }
>> +     np_bat_supply = of_find_node_by_phandle(be32_to_cpup(ph));
> 
> </remove>
> 
> ... and replace with: np_bat_supply = of_parse_phandle(np, "battery-info", 0) instead.
> 
>> +     if (np_bat_supply == NULL) {
> 
> if (!np_bat_supply) {
> 
> I'll not mention this again.
> 
>> +             dev_err(dev, "invalid battery-info node\n");
>> +             return -EINVAL;
>> +     }
>> +     if (of_property_read_bool(np_bat_supply,
>> +                     "thermistor-on-batctrl") == false){
> 
> Replace with:
>         if (of_get_property(np_bat_supply, "thermistor-on-batctr", NULL))
>                 np_bat_supply =  true;
> 
> <remove>
> 
>> +             dev_warn(dev, "missing property thermistor-on-batctrl\n");
>> +             thermistor = NTC_EXTERNAL;
>> +     }
> 
> </remove>
> 
>> +     pdata->battery = &ab8500_bm_data;
>> +     bat = pdata->battery;
> 
> Why not: bat = &ab8500_bm_data
> 
> Or just use ab8500_bm_data in its own right?
> 
>> +     if (thermistor == NTC_EXTERNAL) {
>> +             bat->n_btypes  = 4;
>> +             bat->bat_type  = bat_type_ext_thermistor;
>> +             bat->adc_therm = ABx500_ADC_THERM_BATTEMP;
>> +     }
>> +     ret = of_property_read_string(np_bat_supply, "battery-name", &bat_tech);
>> +     if (ret < 0) {
>> +             dev_warn(dev, "missing property battery-name/type\n");
>> +             bat_tech = "UNKNOWN";
>> +     }
>> +     of_node_put(np_bat_supply);
>> +     if (strcmp(bat_tech, "LION") == 0) {
>> +             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;
>> +     }
>> +     /* select the battery resolution table */
>> +     for (i = 0; i < bat->n_btypes; ++i) {
>> +             btype = (bat->bat_type + i);
>> +             if (thermistor == NTC_EXTERNAL) {
>> +                     btype->batres_tbl =
>> +                             temp_to_batres_tbl_ext_thermistor;
>> +             } else if (strcmp(bat_tech, "LION") == 0) {
> 
> Isn't strncmp safer, since you know the size of the comparison?
> 
>> +                     btype->batres_tbl =
>> +                             temp_to_batres_tbl_9100;
>> +             } else {
>> +                     btype->batres_tbl =
>> +                             temp_to_batres_tbl_thermistor;
>> +             }
>> +     }
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL(bmdevs_of_probe);
> 
> Why are you exporting?
> 
>> diff --git a/drivers/power/ab8500_btemp.c b/drivers/power/ab8500_btemp.c
>> index bba3cca..8e427e7 100644
>> --- a/drivers/power/ab8500_btemp.c
>> +++ b/drivers/power/ab8500_btemp.c
>> @@ -93,7 +93,7 @@ struct ab8500_btemp {
>>       struct ab8500 *parent;
>>       struct ab8500_gpadc *gpadc;
>>       struct ab8500_fg *fg;
>> -     struct abx500_btemp_platform_data *pdata;
>> +     struct abx500_bmdevs_plat_data *pdata;
>>       struct abx500_bm_data *bat;
>>       struct power_supply btemp_psy;
>>       struct ab8500_btemp_events events;
>> @@ -982,7 +982,7 @@ static int __devinit ab8500_btemp_probe(struct platform_device *pdev)
>>       di->gpadc = ab8500_gpadc_get("ab8500-gpadc.0");
>>
>>       /* get btemp specific platform data */
>> -     di->pdata = plat_data->btemp;
>> +     di->pdata = plat_data->bmdev_pdata;
>>       if (!di->pdata) {
>>               dev_err(di->dev, "no btemp platform data supplied\n");
>>               ret = -EINVAL;
>> diff --git a/drivers/power/ab8500_charger.c b/drivers/power/ab8500_charger.c
>> index d4f0c98..5ff0d83 100644
>> --- a/drivers/power/ab8500_charger.c
>> +++ b/drivers/power/ab8500_charger.c
>> @@ -220,7 +220,7 @@ struct ab8500_charger {
>>       bool autopower;
>>       struct ab8500 *parent;
>>       struct ab8500_gpadc *gpadc;
>> -     struct abx500_charger_platform_data *pdata;
>> +     struct abx500_bmdevs_plat_data *pdata;
>>       struct abx500_bm_data *bat;
>>       struct ab8500_charger_event_flags flags;
>>       struct ab8500_charger_usb_state usb_state;
>> @@ -2555,7 +2555,7 @@ static int __devinit ab8500_charger_probe(struct platform_device *pdev)
>>       spin_lock_init(&di->usb_state.usb_lock);
>>
>>       /* get charger specific platform data */
>> -     di->pdata = plat_data->charger;
>> +     di->pdata = plat_data->bmdev_pdata;
>>       if (!di->pdata) {
>>               dev_err(di->dev, "no charger platform data supplied\n");
>>               ret = -EINVAL;
>> diff --git a/drivers/power/ab8500_fg.c b/drivers/power/ab8500_fg.c
>> index bf02225..96741b8 100644
>> --- a/drivers/power/ab8500_fg.c
>> +++ b/drivers/power/ab8500_fg.c
>> @@ -22,15 +22,14 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/power_supply.h>
>>  #include <linux/kobject.h>
>> -#include <linux/mfd/abx500/ab8500.h>
>> -#include <linux/mfd/abx500.h>
>>  #include <linux/slab.h>
>> -#include <linux/mfd/abx500/ab8500-bm.h>
>>  #include <linux/delay.h>
>> -#include <linux/mfd/abx500/ab8500-gpadc.h>
>> -#include <linux/mfd/abx500.h>
>>  #include <linux/time.h>
>>  #include <linux/completion.h>
>> +#include <linux/mfd/abx500.h>
>> +#include <linux/mfd/abx500/ab8500.h>
>> +#include <linux/mfd/abx500/ab8500-bm.h>
>> +#include <linux/mfd/abx500/ab8500-gpadc.h>
>>
>>  #define MILLI_TO_MICRO                       1000
>>  #define FG_LSB_IN_MA                 1627
>> @@ -212,7 +211,7 @@ struct ab8500_fg {
>>       struct ab8500_fg_avg_cap avg_cap;
>>       struct ab8500 *parent;
>>       struct ab8500_gpadc *gpadc;
>> -     struct abx500_fg_platform_data *pdata;
>> +     struct abx500_bmdevs_plat_data *pdata;
>>       struct abx500_bm_data *bat;
>>       struct power_supply fg_psy;
>>       struct workqueue_struct *fg_wq;
>> @@ -544,14 +543,14 @@ cc_err:
>>               ret = abx500_set_register_interruptible(di->dev,
>>                       AB8500_GAS_GAUGE, AB8500_GASG_CC_NCOV_ACCU,
>>                       SEC_TO_SAMPLE(10));
>> -             if (ret)
>> +             if (ret < 0)
> 
> I don't 'think' this change is required. abx500_set_register_interruptible
> will only return !0 on error.
> 
>>                       goto fail;
>>
>>               /* Start the CC */
>>               ret = abx500_set_register_interruptible(di->dev, AB8500_RTC,
>>                       AB8500_RTC_CC_CONF_REG,
>>                       (CC_DEEP_SLEEP_ENA | CC_PWR_UP_ENA));
>> -             if (ret)
>> +             if (ret < 0)
>>                       goto fail;
>>       } else {
>>               di->turn_off_fg = false;
>> @@ -2429,7 +2428,6 @@ static int __devexit ab8500_fg_remove(struct platform_device *pdev)
>>       flush_scheduled_work();
>>       power_supply_unregister(&di->fg_psy);
>>       platform_set_drvdata(pdev, NULL);
>> -     kfree(di);
>>       return ret;
>>  }
>>
>> @@ -2446,18 +2444,47 @@ static int __devinit ab8500_fg_probe(struct platform_device *pdev)
>>  {
>>       int i, irq;
>>       int ret = 0;
>> -     struct abx500_bm_plat_data *plat_data = pdev->dev.platform_data;
>> +     struct abx500_bm_plat_data *plat_data
>> +                             = pdev->dev.platform_data;
>> +     struct device_node *np  = pdev->dev.of_node;
>>       struct ab8500_fg *di;
>>
>> +     di = devm_kzalloc(&pdev->dev, sizeof(*di), GFP_KERNEL);
>> +     if (!di) {
>> +             dev_err(&pdev->dev, "%s no mem for ab8500_fg\n", __func__);
>> +             return -ENOMEM;
>> +     }
>> +     if (np) {
>> +             if (!plat_data) {
> 
> Change these around.
> 
> if (!plat_data) {
>         if (np) {
>                 <snip>
>         } else {
>                 <ERROR>
>         }
> }
> 
>> +                     plat_data =
>> +                     devm_kzalloc(&pdev->dev, sizeof(*plat_data),
>> +                                     GFP_KERNEL);
>> +                     if (!plat_data) {
>> +                             dev_err(&pdev->dev,
>> +                                     "%s no mem for plat_data\n", __func__);
>> +                             return -ENOMEM;
>> +                     }
>> +                     plat_data->bmdev_pdata = devm_kzalloc(&pdev->dev,
>> +                             sizeof(*plat_data->bmdev_pdata), GFP_KERNEL);
>> +                     if (!plat_data->bmdev_pdata) {
>> +                             dev_err(&pdev->dev,
>> +                                     "%s no mem for pdata->fg\n",
>> +                                     __func__);
>> +                             return -ENOMEM;
>> +                     }
>> +             }
>> +             ret = bmdevs_of_probe(&pdev->dev, np, plat_data);
>> +             if (ret < 0) {
>> +                     dev_err(&pdev->dev, "failed to get platform data\n");
>> +                     return ret;
>> +             }
>> +     }
> 
> <remove>
> 
>>       if (!plat_data) {
>> -             dev_err(&pdev->dev, "No platform data\n");
>> +             dev_err(&pdev->dev,
>> +                     "%s no fg platform data found\n", __func__);
>>               return -EINVAL;
>>       }
> </remove>
> 
>> -     di = kzalloc(sizeof(*di), GFP_KERNEL);
>> -     if (!di)
>> -             return -ENOMEM;
>> -
>>       mutex_init(&di->cc_lock);
>>
>>       /* get parent data */
>> @@ -2466,19 +2493,17 @@ static int __devinit ab8500_fg_probe(struct platform_device *pdev)
>>       di->gpadc = ab8500_gpadc_get("ab8500-gpadc.0");
>>
>>       /* get fg specific platform data */
>> -     di->pdata = plat_data->fg;
>> +     di->pdata = plat_data->bmdev_pdata;
>>       if (!di->pdata) {
>>               dev_err(di->dev, "no fg platform data supplied\n");
>> -             ret = -EINVAL;
>> -             goto free_device_info;
>> +             return -EINVAL;
>>       }
>>
>>       /* get battery specific platform data */
>>       di->bat = plat_data->battery;
>>       if (!di->bat) {
>>               dev_err(di->dev, "no battery platform data supplied\n");
>> -             ret = -EINVAL;
>> -             goto free_device_info;
>> +             return -EINVAL;
>>       }
>>
>>       di->fg_psy.name = "ab8500_fg";
>> @@ -2506,7 +2531,7 @@ static int __devinit ab8500_fg_probe(struct platform_device *pdev)
>>       di->fg_wq = create_singlethread_workqueue("ab8500_fg_wq");
>>       if (di->fg_wq == NULL) {
>>               dev_err(di->dev, "failed to create work queue\n");
>> -             goto free_device_info;
>> +             return -ENOMEM;
>>       }
>>
>>       /* Init work for running the fg algorithm instantly */
>> @@ -2605,12 +2630,14 @@ free_irq:
>>       }
>>  free_inst_curr_wq:
>>       destroy_workqueue(di->fg_wq);
>> -free_device_info:
>> -     kfree(di);
>> -
>>       return ret;
>>  }
>>
>> +static const struct of_device_id ab8500_fg_match[] = {
>> +     {.compatible = "stericsson,ab8500-fg",},
> 
> <nit>
> 
> Spaces:
> 
> { .compatible = "stericsson,ab8500-fg", },
> 
> </nit>
> 
>> +     {},
>> +};
>> +
>>  static struct platform_driver ab8500_fg_driver = {
>>       .probe = ab8500_fg_probe,
>>       .remove = __devexit_p(ab8500_fg_remove),
>> @@ -2619,6 +2646,7 @@ static struct platform_driver ab8500_fg_driver = {
>>       .driver = {
>>               .name = "ab8500-fg",
>>               .owner = THIS_MODULE,
>> +             .of_match_table = ab8500_fg_match,
>>       },
>>  };
>>
>> diff --git a/drivers/power/abx500_chargalg.c b/drivers/power/abx500_chargalg.c
>> index 804b88c..ba548e4 100644
>> --- a/drivers/power/abx500_chargalg.c
>> +++ b/drivers/power/abx500_chargalg.c
>> @@ -231,7 +231,7 @@ struct abx500_chargalg {
>>       struct abx500_chargalg_charger_info chg_info;
>>       struct abx500_chargalg_battery_data batt_data;
>>       struct abx500_chargalg_suspension_status susp_status;
>> -     struct abx500_chargalg_platform_data *pdata;
>> +     struct abx500_bmdevs_plat_data *pdata;
>>       struct abx500_bm_data *bat;
>>       struct power_supply chargalg_psy;
>>       struct ux500_charger *ac_chg;
>> @@ -1814,7 +1814,7 @@ static int __devinit abx500_chargalg_probe(struct platform_device *pdev)
>>       di->dev = &pdev->dev;
>>
>>       plat_data = pdev->dev.platform_data;
>> -     di->pdata = plat_data->chargalg;
>> +     di->pdata = plat_data->bmdev_pdata;
>>       di->bat = plat_data->battery;
>>
>>       /* chargalg supply */
>> diff --git a/include/linux/mfd/abx500.h b/include/linux/mfd/abx500.h
>> index 1318ca6..286f8ac 100644
>> --- a/include/linux/mfd/abx500.h
>> +++ b/include/linux/mfd/abx500.h
>> @@ -382,39 +382,30 @@ struct abx500_bm_data {
>>       int gnd_lift_resistance;
>>       const struct abx500_maxim_parameters *maxi;
>>       const struct abx500_bm_capacity_levels *cap_levels;
>> -     const struct abx500_battery_type *bat_type;
>> +     struct abx500_battery_type *bat_type;
>>       const struct abx500_bm_charger_parameters *chg_params;
>>       const struct abx500_fg_parameters *fg_params;
>>  };
>>
>> -struct abx500_chargalg_platform_data {
>> -     char **supplied_to;
>> -     size_t num_supplicants;
>> +struct abx500_bmdevs_plat_data {
>> +     char    **supplied_to;
>> +     size_t  num_supplicants;
>> +     bool    autopower_cfg;
>>  };
>>
>> -struct abx500_charger_platform_data {
>> -     char **supplied_to;
>> -     size_t num_supplicants;
>> -     bool autopower_cfg;
>> -};
>> -
>> -struct abx500_btemp_platform_data {
>> -     char **supplied_to;
>> -     size_t num_supplicants;
>> +struct abx500_bm_plat_data {
>> +     struct abx500_bm_data *battery;
>> +     struct abx500_bmdevs_plat_data *bmdev_pdata;
>>  };
>>
>> -struct abx500_fg_platform_data {
>> -     char **supplied_to;
>> -     size_t num_supplicants;
>> +enum {
>> +     NTC_EXTERNAL = 0,
>> +     NTC_INTERNAL,
>>  };
>>
>> -struct abx500_bm_plat_data {
>> -     struct abx500_bm_data *battery;
>> -     struct abx500_charger_platform_data *charger;
>> -     struct abx500_btemp_platform_data *btemp;
>> -     struct abx500_fg_platform_data *fg;
>> -     struct abx500_chargalg_platform_data *chargalg;
>> -};
>> +int bmdevs_of_probe(struct device *dev,
>> +             struct device_node *np,
>> +             struct abx500_bm_plat_data *pdata);
>>
>>  int abx500_set_register_interruptible(struct device *dev, u8 bank, u8 reg,
>>       u8 value);
>> diff --git a/include/linux/mfd/abx500/ab8500-bm.h b/include/linux/mfd/abx500/ab8500-bm.h
>> index 44310c9..d15b7f1 100644
>> --- a/include/linux/mfd/abx500/ab8500-bm.h
>> +++ b/include/linux/mfd/abx500/ab8500-bm.h
>> @@ -422,6 +422,13 @@ struct ab8500_chargalg_platform_data {
>>  struct ab8500_btemp;
>>  struct ab8500_gpadc;
>>  struct ab8500_fg;
>> +
>> +extern struct abx500_bm_data ab8500_bm_data;
>> +extern struct abx500_battery_type bat_type_ext_thermistor[];
>> +extern struct batres_vs_temp temp_to_batres_tbl_ext_thermistor[];
>> +extern struct batres_vs_temp temp_to_batres_tbl_9100[];
>> +extern struct batres_vs_temp temp_to_batres_tbl_thermistor[];
>> +
>>  #ifdef CONFIG_AB8500_BM
>>  void ab8500_fg_reinit(void);
>>  void ab8500_charger_usb_state_changed(u8 bm_usb_state, u16 mA);
>> --
>> 1.7.9.5
>>
> 
> --
> Lee Jones
> Linaro ST-Ericsson Landing Team Lead
> Linaro.org ? Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
>

Lee Jones Oct. 1, 2012, 10:22 a.m. UTC | #3

Sorry, some mistakes:

> > From: "Rajanikanth H.V" <rajanikanth.hv@stericsson.com>
> > 
> > - This patch adds device tree support for fuelguage driver
> > - optimize bm devices platform_data usage and of_probe(...)
> >   Note: of_probe() routine for battery managed devices is made
> >   common across all bm drivers.

Spelling errors in here.

> > +		dev_err(dev, "invalid battery-info node\n");
> > +		return -EINVAL;
> > +	}
> > +	if (of_property_read_bool(np_bat_supply,
> > +			"thermistor-on-batctrl") == false){
> 
> Replace with: 
>         if (of_get_property(np_bat_supply, "thermistor-on-batctr", NULL))
> 	        np_bat_supply =  true;

This should be: 

         if (of_get_property(np_bat_supply, "thermistor-on-batctr", NULL))
 	        thermistor = NTC_INTERNAL;
         else
                thermistor = NTC_EXTERNAL;

> <remove>
> 
> > +		dev_warn(dev, "missing property thermistor-on-batctrl\n");
> > +		thermistor = NTC_EXTERNAL;
> > +	}
> 
> </remove>

Lee Jones Oct. 1, 2012, 10:36 a.m. UTC | #4

> did you have a look at arnd and anton comments regarding
> 'supplied-to' and boolean property

Try to keep your comments inline, situated below the relevant comment.

> >> +     ab8500_battery_info: ab8500_bat_type {
> >> +             battery-type = <2>;
> >> +             thermistor-on-batctrl = <1>;

> > You have this as a bool here, and ...

> >> +     e.g:
> >> +     ab8500_battery_info: ab8500_bat_type {
> >> +             thermistor-on-batctrl;

> > ... a standard property here. I suggest you drop the bool value.

I'm guessing it's just the documentation that needs amending.

Francesco Lavra Oct. 6, 2012, 2:01 p.m. UTC | #5

Hi,

On 10/01/2012 06:08 AM, Rajanikanth H.V wrote:
> From: "Rajanikanth H.V" <rajanikanth.hv@stericsson.com>
> 
> - This patch adds device tree support for fuelguage driver
> - optimize bm devices platform_data usage and of_probe(...)
>   Note: of_probe() routine for battery managed devices is made
>   common across all bm drivers.
[...]
> +int __devinit
> +bmdevs_of_probe(struct device *dev,
> +		struct device_node *np,
> +		struct abx500_bm_plat_data *pdata)
> +{
> +	int	i, ret = 0, thermistor = NTC_INTERNAL;
> +	const	__be32 *ph;
> +	const	char *bat_tech;
> +	struct	abx500_bm_data		 *bat;
> +	struct	abx500_battery_type	 *btype;
> +	struct  device_node		 *np_bat_supply;
> +	struct  abx500_bmdevs_plat_data  *plat_data = pdata->bmdev_pdata;
> +
> +	/* get phandle to 'supplied-to' node */
> +	ph = of_get_property(np, "supplied-to", &plat_data->num_supplicants);
> +	if (ph == NULL) {
> +		dev_err(dev, "no supplied_to property specified\n");
> +		return -EINVAL;
> +	}
> +	plat_data->num_supplicants /= sizeof(int);
> +	plat_data->supplied_to =
> +		devm_kzalloc(dev, plat_data->num_supplicants *
> +			sizeof(const char *), GFP_KERNEL);
> +	if (plat_data->supplied_to == NULL) {
> +		dev_err(dev, "%s no mem for supplied-to\n", __func__);
> +		return -ENOMEM;
> +	}
> +	for (i = 0; i < plat_data->num_supplicants; ++i) {
> +		np_bat_supply = of_find_node_by_phandle(be32_to_cpup(ph) + i);
> +		if (np_bat_supply == NULL) {
> +			dev_err(dev, "invalid supplied_to property\n");
> +			return -EINVAL;
> +		}
> +		ret = of_property_read_string(np_bat_supply, "interface-name",
> +				(const char **)(plat_data->supplied_to + i));
> +		if (ret < 0) {
> +			of_node_put(np_bat_supply);
> +			dev_err(dev, "supply/interface name not found\n");
> +			return ret;
> +		}

If an error is encountered here, of_node_put() should be called for all
nodes previously referenced with of_find_node_by_phandle (or
of_parse_phandle, as Lee suggested).
Also, if I'm not mistaken we have a leak here, because the refcount of
these nodes is never decremented, not even in the driver remove routine.

[...]
> @@ -2446,18 +2444,47 @@ static int __devinit ab8500_fg_probe(struct platform_device *pdev)
>  {
>  	int i, irq;
>  	int ret = 0;
> -	struct abx500_bm_plat_data *plat_data = pdev->dev.platform_data;
> +	struct abx500_bm_plat_data *plat_data
> +				= pdev->dev.platform_data;
> +	struct device_node *np	= pdev->dev.of_node;
>  	struct ab8500_fg *di;
>  
> +	di = devm_kzalloc(&pdev->dev, sizeof(*di), GFP_KERNEL);
> +	if (!di) {
> +		dev_err(&pdev->dev, "%s no mem for ab8500_fg\n", __func__);
> +		return -ENOMEM;
> +	}
> +	if (np) {
> +		if (!plat_data) {
> +			plat_data =
> +			devm_kzalloc(&pdev->dev, sizeof(*plat_data),
> +					GFP_KERNEL);
> +			if (!plat_data) {
> +				dev_err(&pdev->dev,
> +					"%s no mem for plat_data\n", __func__);
> +				return -ENOMEM;
> +			}
> +			plat_data->bmdev_pdata = devm_kzalloc(&pdev->dev,
> +				sizeof(*plat_data->bmdev_pdata), GFP_KERNEL);
> +			if (!plat_data->bmdev_pdata) {
> +				dev_err(&pdev->dev,
> +					"%s no mem for pdata->fg\n",
> +					__func__);
> +				return -ENOMEM;
> +			}
> +		}
> +		ret = bmdevs_of_probe(&pdev->dev, np, plat_data);

I think it's better to move allocation of bmdev_pdata and corresponding
error check to bmdevs_of_probe(), because this code is shared by all
battery management drivers.

--
Francesco

[1/4] mfd: ab8500: add devicetree support for fuelgauge

Commit Message

Comments

Patch