@@ -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
@@ -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
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.
Boolean properties should be empty when enabled and not present when
disabled. In this example, one would just write
thermister-internal-to-battery;
Arnd
On Monday 10 September 2012 07:31 PM, Arnd Bergmann wrote:
On Monday 10 September 2012, Rajanikanth HV wrote:
quoted
+
+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?
We discussed about this on : "13 July 2012 17:05", pasting from that
mail thread.
============================
quoted
+Supplied-to:
+ This shall be power supply class dependency where in the
runtime battery
quoted
+ 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?
============================
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.
Boolean properties should be empty when enabled and not present when
disabled. In this example, one would just write
thermister-internal-to-battery;
Arnd
On Tuesday 11 September 2012, Rajanikanth HV wrote:
quoted
quoted
+Supplied-to:
+ This shall be power supply class dependency where in the
runtime battery
quoted
quoted
+ properties will be shared across fuel guage and charging
algorithm driver.
quoted
I probably don't understand enough of this, but shouldn't the other
devices
quoted
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
On Tuesday 11 September 2012 04:52 PM, Arnd Bergmann wrote:
On Tuesday 11 September 2012, Rajanikanth HV wrote:
quoted
quoted
quoted
+Supplied-to:
+ This shall be power supply class dependency where in the
runtime battery
quoted
quoted
+ properties will be shared across fuel guage and charging
algorithm driver.
quoted
I probably don't understand enough of this, but shouldn't the other
devices
quoted
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
On Wednesday 12 September 2012, Rajanikanth HV wrote:
On Tuesday 11 September 2012 04:52 PM, Arnd Bergmann wrote:
quoted
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
On Wednesday 12 September 2012 09:06 PM, Arnd Bergmann wrote:
On Wednesday 12 September 2012, Rajanikanth HV wrote:
quoted
On Tuesday 11 September 2012 04:52 PM, Arnd Bergmann wrote:
quoted
On Tuesday 11 September 2012, Rajanikanth HV wrote:
quoted
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.
On Thursday 13 September 2012, Rajanikanth HV wrote:
On Wednesday 12 September 2012 09:06 PM, Arnd Bergmann wrote:> >
quoted
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?
quoted
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
quoted
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
From: Anton Vorontsov <hidden> Date: 2012-09-14 02:07:07
(Thanks for Cc'ing me.)
On Thu, Sep 13, 2012 at 02:37:38PM +0000, Arnd Bergmann wrote:
[...]
quoted
quoted
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.
[...]
quoted
quoted
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.
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 at ... {
...
};
usb-power: power at ... {
...
};
main-battery: battery at ... {
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
On Friday 14 September 2012 01:39 PM, Arnd Bergmann wrote:
On Friday 14 September 2012, Anton Vorontsov wrote:
quoted
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 at ... {
...
};
usb-power: power at ... {
...
};
main-battery: battery at ... {
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
On Thursday 13 September 2012 08:07 PM, Arnd Bergmann wrote:
On Thursday 13 September 2012, Rajanikanth HV wrote:
quoted
On Wednesday 12 September 2012 09:06 PM, Arnd Bergmann wrote:> >
quoted
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?
quoted
quoted
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.
quoted
For this, I would suggest you give a list of
quoted
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.
From: Francesco Lavra <hidden> Date: 2012-09-15 11:00:35
On 09/10/2012 01:21 PM, Rajanikanth HV wrote:
...
quoted hunk
-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
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