* [PATCH 01/11] power: supply: ab8500: Drop BATCTRL thermal mode
2022-01-27 1:12 [PATCH 00/11] AB8500 charging fixes Linus Walleij
@ 2022-01-27 1:12 ` Linus Walleij
2022-01-27 1:12 ` [PATCH 02/11] power: supply: ab8500: Swap max and overvoltage Linus Walleij
` (9 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2022-01-27 1:12 UTC (permalink / raw)
To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Linus Walleij
The BATCTRL mode reads the temperature of the battery by
enabling a certain probing current (7-20 mA) and then measure
the voltage of the NTC mounted inside the battery.
None of the AB8500 product or the reference designs use this
mode. What we use is the so-called BATTEMP mode which enables
an internal 230 kOhm pull-up to 1.8 V to the external NTC on
pin BatTemp (N16) and then measures the voltage over the NTC
using the ADC:
1.8V (VTVOUT)
|
[ ] 230 kOhm
|
BatTemp +---------------- ADC
Pin N16 | _
|/
[/] NTC
_/|
|
GND
Cut out the BATCTRL code to clear the forest and stop
maintaining code we can never test.
The current inducing method is still used to probe for the
battery type using the internal BTI (battery type indicator)
on the BatCtrl (C3) pin in a separate code path.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/power/supply/ab8500-bm.h | 16 --
drivers/power/supply/ab8500_bmdata.c | 2 -
drivers/power/supply/ab8500_btemp.c | 263 +++------------------------
3 files changed, 24 insertions(+), 257 deletions(-)
diff --git a/drivers/power/supply/ab8500-bm.h b/drivers/power/supply/ab8500-bm.h
index 56a5aaf9a27a..3bc6fd9337d2 100644
--- a/drivers/power/supply/ab8500-bm.h
+++ b/drivers/power/supply/ab8500-bm.h
@@ -260,18 +260,6 @@ enum bup_vch_sel {
#define BUS_PP_PRECHG_CURRENT_MASK 0x0E
#define BUS_POWER_PATH_PRECHG_ENA 0x01
-/*
- * ADC for the battery thermistor.
- * When using the AB8500_ADC_THERM_BATCTRL the battery ID resistor is combined
- * with a NTC resistor to both identify the battery and to measure its
- * temperature. Different phone manufactures uses different techniques to both
- * identify the battery and to read its temperature.
- */
-enum ab8500_adc_therm {
- AB8500_ADC_THERM_BATCTRL,
- AB8500_ADC_THERM_BATTEMP,
-};
-
/**
* struct ab8500_res_to_temp - defines one point in a temp to res curve. To
* be used in battery packs that combines the identification resistor with a
@@ -423,7 +411,6 @@ struct ab8500_bm_charger_parameters {
* @bkup_bat_i current which we charge the backup battery with
* @no_maintenance indicates that maintenance charging is disabled
* @capacity_scaling indicates whether capacity scaling is to be used
- * @ab8500_adc_therm placement of thermistor, batctrl or battemp adc
* @chg_unknown_bat flag to enable charging of unknown batteries
* @enable_overshoot flag to enable VBAT overshoot control
* @auto_trig flag to enable auto adc trigger
@@ -431,7 +418,6 @@ struct ab8500_bm_charger_parameters {
* @interval_charging charge alg cycle period time when charging (sec)
* @interval_not_charging charge alg cycle period time when not charging (sec)
* @temp_hysteresis temperature hysteresis
- * @gnd_lift_resistance Battery ground to phone ground resistance (mOhm)
* @maxi maximization parameters
* @cap_levels capacity in percent for the different capacity levels
* @bat_type table of supported battery types
@@ -452,12 +438,10 @@ struct ab8500_bm_data {
bool chg_unknown_bat;
bool enable_overshoot;
bool auto_trig;
- enum ab8500_adc_therm adc_therm;
int fg_res;
int interval_charging;
int interval_not_charging;
int temp_hysteresis;
- int gnd_lift_resistance;
const struct ab8500_maxim_parameters *maxi;
const struct ab8500_bm_capacity_levels *cap_levels;
struct ab8500_battery_type *bat_type;
diff --git a/drivers/power/supply/ab8500_bmdata.c b/drivers/power/supply/ab8500_bmdata.c
index 7ae95f537580..7133cce6a25a 100644
--- a/drivers/power/supply/ab8500_bmdata.c
+++ b/drivers/power/supply/ab8500_bmdata.c
@@ -150,7 +150,6 @@ struct ab8500_bm_data ab8500_bm_data = {
.bkup_bat_i = BUP_ICH_SEL_150UA,
.no_maintenance = false,
.capacity_scaling = false,
- .adc_therm = AB8500_ADC_THERM_BATCTRL,
.chg_unknown_bat = false,
.enable_overshoot = false,
.fg_res = 100,
@@ -158,7 +157,6 @@ struct ab8500_bm_data ab8500_bm_data = {
.bat_type = &bat_type_thermistor_unknown,
.interval_charging = 5,
.interval_not_charging = 120,
- .gnd_lift_resistance = 34,
.maxi = &ab8500_maxi_params,
.chg_params = &chg,
.fg_params = &fg,
diff --git a/drivers/power/supply/ab8500_btemp.c b/drivers/power/supply/ab8500_btemp.c
index cc33c5187fbb..a5ca09124c93 100644
--- a/drivers/power/supply/ab8500_btemp.c
+++ b/drivers/power/supply/ab8500_btemp.c
@@ -135,8 +135,6 @@ static LIST_HEAD(ab8500_btemp_list);
static int ab8500_btemp_batctrl_volt_to_res(struct ab8500_btemp *di,
int v_batctrl, int inst_curr)
{
- int rbs;
-
if (is_ab8500_1p1_or_earlier(di->parent)) {
/*
* For ABB cut1.0 and 1.1 BAT_CTRL is internally
@@ -145,23 +143,11 @@ static int ab8500_btemp_batctrl_volt_to_res(struct ab8500_btemp *di,
return (450000 * (v_batctrl)) / (1800 - v_batctrl);
}
- if (di->bm->adc_therm == AB8500_ADC_THERM_BATCTRL) {
- /*
- * If the battery has internal NTC, we use the current
- * source to calculate the resistance.
- */
- rbs = (v_batctrl * 1000
- - di->bm->gnd_lift_resistance * inst_curr)
- / di->curr_source;
- } else {
- /*
- * BAT_CTRL is internally
- * connected to 1.8V through a 80k resistor
- */
- rbs = (80000 * (v_batctrl)) / (1800 - v_batctrl);
- }
-
- return rbs;
+ /*
+ * BAT_CTRL is internally
+ * connected to 1.8V through a 80k resistor
+ */
+ return (80000 * (v_batctrl)) / (1800 - v_batctrl);
}
/**
@@ -186,155 +172,6 @@ static int ab8500_btemp_read_batctrl_voltage(struct ab8500_btemp *di)
return vbtemp;
}
-/**
- * ab8500_btemp_curr_source_enable() - enable/disable batctrl current source
- * @di: pointer to the ab8500_btemp structure
- * @enable: enable or disable the current source
- *
- * Enable or disable the current sources for the BatCtrl AD channel
- */
-static int ab8500_btemp_curr_source_enable(struct ab8500_btemp *di,
- bool enable)
-{
- int curr;
- int ret = 0;
-
- /*
- * BATCTRL current sources are included on AB8500 cut2.0
- * and future versions
- */
- if (is_ab8500_1p1_or_earlier(di->parent))
- return 0;
-
- /* Only do this for batteries with internal NTC */
- if (di->bm->adc_therm == AB8500_ADC_THERM_BATCTRL && enable) {
-
- if (di->curr_source == BTEMP_BATCTRL_CURR_SRC_7UA)
- curr = BAT_CTRL_7U_ENA;
- else
- curr = BAT_CTRL_20U_ENA;
-
- dev_dbg(di->dev, "Set BATCTRL %duA\n", di->curr_source);
-
- ret = abx500_mask_and_set_register_interruptible(di->dev,
- AB8500_CHARGER, AB8500_BAT_CTRL_CURRENT_SOURCE,
- FORCE_BAT_CTRL_CMP_HIGH, FORCE_BAT_CTRL_CMP_HIGH);
- if (ret) {
- dev_err(di->dev, "%s failed setting cmp_force\n",
- __func__);
- return ret;
- }
-
- /*
- * We have to wait one 32kHz cycle before enabling
- * the current source, since ForceBatCtrlCmpHigh needs
- * to be written in a separate cycle
- */
- udelay(32);
-
- ret = abx500_set_register_interruptible(di->dev,
- AB8500_CHARGER, AB8500_BAT_CTRL_CURRENT_SOURCE,
- FORCE_BAT_CTRL_CMP_HIGH | curr);
- if (ret) {
- dev_err(di->dev, "%s failed enabling current source\n",
- __func__);
- goto disable_curr_source;
- }
- } else if (di->bm->adc_therm == AB8500_ADC_THERM_BATCTRL && !enable) {
- dev_dbg(di->dev, "Disable BATCTRL curr source\n");
-
- /* Write 0 to the curr bits */
- ret = abx500_mask_and_set_register_interruptible(
- di->dev,
- AB8500_CHARGER, AB8500_BAT_CTRL_CURRENT_SOURCE,
- BAT_CTRL_7U_ENA | BAT_CTRL_20U_ENA,
- ~(BAT_CTRL_7U_ENA | BAT_CTRL_20U_ENA));
-
- if (ret) {
- dev_err(di->dev, "%s failed disabling current source\n",
- __func__);
- goto disable_curr_source;
- }
-
- /* Enable Pull-Up and comparator */
- ret = abx500_mask_and_set_register_interruptible(di->dev,
- AB8500_CHARGER, AB8500_BAT_CTRL_CURRENT_SOURCE,
- BAT_CTRL_PULL_UP_ENA | BAT_CTRL_CMP_ENA,
- BAT_CTRL_PULL_UP_ENA | BAT_CTRL_CMP_ENA);
- if (ret) {
- dev_err(di->dev, "%s failed enabling PU and comp\n",
- __func__);
- goto enable_pu_comp;
- }
-
- /*
- * We have to wait one 32kHz cycle before disabling
- * ForceBatCtrlCmpHigh since this needs to be written
- * in a separate cycle
- */
- udelay(32);
-
- /* Disable 'force comparator' */
- ret = abx500_mask_and_set_register_interruptible(di->dev,
- AB8500_CHARGER, AB8500_BAT_CTRL_CURRENT_SOURCE,
- FORCE_BAT_CTRL_CMP_HIGH, ~FORCE_BAT_CTRL_CMP_HIGH);
- if (ret) {
- dev_err(di->dev, "%s failed disabling force comp\n",
- __func__);
- goto disable_force_comp;
- }
- }
- return ret;
-
- /*
- * We have to try unsetting FORCE_BAT_CTRL_CMP_HIGH one more time
- * if we got an error above
- */
-disable_curr_source:
- /* Write 0 to the curr bits */
- ret = abx500_mask_and_set_register_interruptible(di->dev,
- AB8500_CHARGER, AB8500_BAT_CTRL_CURRENT_SOURCE,
- BAT_CTRL_7U_ENA | BAT_CTRL_20U_ENA,
- ~(BAT_CTRL_7U_ENA | BAT_CTRL_20U_ENA));
-
- if (ret) {
- dev_err(di->dev, "%s failed disabling current source\n",
- __func__);
- return ret;
- }
-enable_pu_comp:
- /* Enable Pull-Up and comparator */
- ret = abx500_mask_and_set_register_interruptible(di->dev,
- AB8500_CHARGER, AB8500_BAT_CTRL_CURRENT_SOURCE,
- BAT_CTRL_PULL_UP_ENA | BAT_CTRL_CMP_ENA,
- BAT_CTRL_PULL_UP_ENA | BAT_CTRL_CMP_ENA);
- if (ret) {
- dev_err(di->dev, "%s failed enabling PU and comp\n",
- __func__);
- return ret;
- }
-
-disable_force_comp:
- /*
- * We have to wait one 32kHz cycle before disabling
- * ForceBatCtrlCmpHigh since this needs to be written
- * in a separate cycle
- */
- udelay(32);
-
- /* Disable 'force comparator' */
- ret = abx500_mask_and_set_register_interruptible(di->dev,
- AB8500_CHARGER, AB8500_BAT_CTRL_CURRENT_SOURCE,
- FORCE_BAT_CTRL_CMP_HIGH, ~FORCE_BAT_CTRL_CMP_HIGH);
- if (ret) {
- dev_err(di->dev, "%s failed disabling force comp\n",
- __func__);
- return ret;
- }
-
- return ret;
-}
-
/**
* ab8500_btemp_get_batctrl_res() - get battery resistance
* @di: pointer to the ab8500_btemp structure
@@ -350,16 +187,6 @@ static int ab8500_btemp_get_batctrl_res(struct ab8500_btemp *di)
int inst_curr;
int i;
- /*
- * BATCTRL current sources are included on AB8500 cut2.0
- * and future versions
- */
- ret = ab8500_btemp_curr_source_enable(di, true);
- if (ret) {
- dev_err(di->dev, "%s curr source enabled failed\n", __func__);
- return ret;
- }
-
if (!di->fg)
di->fg = ab8500_fg_get();
if (!di->fg) {
@@ -395,12 +222,6 @@ static int ab8500_btemp_get_batctrl_res(struct ab8500_btemp *di)
res = ab8500_btemp_batctrl_volt_to_res(di, batctrl, inst_curr);
- ret = ab8500_btemp_curr_source_enable(di, false);
- if (ret) {
- dev_err(di->dev, "%s curr source disable failed\n", __func__);
- return ret;
- }
-
dev_dbg(di->dev, "%s batctrl: %d res: %d inst_curr: %d samples: %d\n",
__func__, batctrl, res, inst_curr, i);
@@ -451,47 +272,28 @@ static int ab8500_btemp_res_to_temp(struct ab8500_btemp *di,
*/
static int ab8500_btemp_measure_temp(struct ab8500_btemp *di)
{
- struct power_supply_battery_info *bi = di->bm->bi;
int temp, ret;
static int prev;
- int rbat, rntc, vntc;
-
- if ((di->bm->adc_therm == AB8500_ADC_THERM_BATCTRL) &&
- (bi && (bi->technology == POWER_SUPPLY_TECHNOLOGY_UNKNOWN))) {
-
- rbat = ab8500_btemp_get_batctrl_res(di);
- if (rbat < 0) {
- dev_err(di->dev, "%s get batctrl res failed\n",
- __func__);
- /*
- * Return out-of-range temperature so that
- * charging is stopped
- */
- return BTEMP_THERMAL_LOW_LIMIT;
- }
-
- temp = ab8500_btemp_res_to_temp(di,
- di->bm->bat_type->r_to_t_tbl,
- di->bm->bat_type->n_temp_tbl_elements, rbat);
- } else {
- ret = iio_read_channel_processed(di->btemp_ball, &vntc);
- if (ret < 0) {
- dev_err(di->dev,
- "%s ADC conversion failed,"
- " using previous value\n", __func__);
- return prev;
- }
- /*
- * The PCB NTC is sourced from VTVOUT via a 230kOhm
- * resistor.
- */
- rntc = 230000 * vntc / (VTVOUT_V - vntc);
+ int rntc, vntc;
- temp = ab8500_btemp_res_to_temp(di,
- di->bm->bat_type->r_to_t_tbl,
- di->bm->bat_type->n_temp_tbl_elements, rntc);
- prev = temp;
+ ret = iio_read_channel_processed(di->btemp_ball, &vntc);
+ if (ret < 0) {
+ dev_err(di->dev,
+ "%s ADC conversion failed,"
+ " using previous value\n", __func__);
+ return prev;
}
+ /*
+ * The PCB NTC is sourced from VTVOUT via a 230kOhm
+ * resistor.
+ */
+ rntc = 230000 * vntc / (VTVOUT_V - vntc);
+
+ temp = ab8500_btemp_res_to_temp(di,
+ di->bm->bat_type->r_to_t_tbl,
+ di->bm->bat_type->n_temp_tbl_elements, rntc);
+ prev = temp;
+
dev_dbg(di->dev, "Battery temperature is %d\n", temp);
return temp;
}
@@ -519,11 +321,9 @@ static int ab8500_btemp_id(struct ab8500_btemp *di)
if ((res <= di->bm->bat_type->resis_high) &&
(res >= di->bm->bat_type->resis_low)) {
- dev_info(di->dev, "Battery detected on %s"
+ dev_info(di->dev, "Battery detected on BATTEMP"
" low %d < res %d < high: %d"
" index: %d\n",
- di->bm->adc_therm == AB8500_ADC_THERM_BATCTRL ?
- "BATCTRL" : "BATTEMP",
di->bm->bat_type->resis_low, res,
di->bm->bat_type->resis_high, i);
} else {
@@ -532,21 +332,6 @@ static int ab8500_btemp_id(struct ab8500_btemp *di)
return -ENXIO;
}
- /*
- * We only have to change current source if the
- * detected type is Type 1 (LIPO) resis_high = 53407, resis_low = 12500
- * if someone hacks this in.
- *
- * FIXME: make sure this is done automatically for the batteries
- * that need it.
- */
- if ((di->bm->adc_therm == AB8500_ADC_THERM_BATCTRL) &&
- (di->bm->bi && (di->bm->bi->technology == POWER_SUPPLY_TECHNOLOGY_LIPO)) &&
- (res <= 53407) && (res >= 12500)) {
- dev_dbg(di->dev, "Set BATCTRL current source to 20uA\n");
- di->curr_source = BTEMP_BATCTRL_CURR_SRC_20UA;
- }
-
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 02/11] power: supply: ab8500: Swap max and overvoltage
2022-01-27 1:12 [PATCH 00/11] AB8500 charging fixes Linus Walleij
2022-01-27 1:12 ` [PATCH 01/11] power: supply: ab8500: Drop BATCTRL thermal mode Linus Walleij
@ 2022-01-27 1:12 ` Linus Walleij
2022-01-27 1:12 ` [PATCH 03/11] power: supply: ab8500: Integrate thermal zone Linus Walleij
` (8 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2022-01-27 1:12 UTC (permalink / raw)
To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Linus Walleij
We should terminate charging when we reach the voltage_max_design_uv
not overvoltage_limit_uv, this is an abuse of that struct member.
The overvoltage limit is actually not configurable on the AB8500,
it is fixed to 4.75 V so drop a comment about that in the code.
Fixes: 2a5f41830aad ("power: supply: ab8500: Standardize voltages")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/power/supply/ab8500_bmdata.c | 8 +++-----
drivers/power/supply/ab8500_chargalg.c | 2 +-
drivers/power/supply/ab8500_fg.c | 8 +++++++-
3 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/power/supply/ab8500_bmdata.c b/drivers/power/supply/ab8500_bmdata.c
index 7133cce6a25a..62b63f0437dd 100644
--- a/drivers/power/supply/ab8500_bmdata.c
+++ b/drivers/power/supply/ab8500_bmdata.c
@@ -186,13 +186,11 @@ int ab8500_bm_of_probe(struct power_supply *psy,
* fall back to safe defaults.
*/
if ((bi->voltage_min_design_uv < 0) ||
- (bi->voltage_max_design_uv < 0) ||
- (bi->overvoltage_limit_uv < 0)) {
+ (bi->voltage_max_design_uv < 0)) {
/* Nominal voltage is 3.7V for unknown batteries */
bi->voltage_min_design_uv = 3700000;
- bi->voltage_max_design_uv = 3700000;
- /* Termination voltage (overcharge limit) 4.05V */
- bi->overvoltage_limit_uv = 4050000;
+ /* Termination voltage 4.05V */
+ bi->voltage_max_design_uv = 4050000;
}
if (bi->constant_charge_current_max_ua < 0)
diff --git a/drivers/power/supply/ab8500_chargalg.c b/drivers/power/supply/ab8500_chargalg.c
index c4a2fe07126c..bcf85ae6828e 100644
--- a/drivers/power/supply/ab8500_chargalg.c
+++ b/drivers/power/supply/ab8500_chargalg.c
@@ -802,7 +802,7 @@ static void ab8500_chargalg_end_of_charge(struct ab8500_chargalg *di)
if (di->charge_status == POWER_SUPPLY_STATUS_CHARGING &&
di->charge_state == STATE_NORMAL &&
!di->maintenance_chg && (di->batt_data.volt_uv >=
- di->bm->bi->overvoltage_limit_uv ||
+ di->bm->bi->voltage_max_design_uv ||
di->events.usb_cv_active || di->events.ac_cv_active) &&
di->batt_data.avg_curr_ua <
di->bm->bi->charge_term_current_ua &&
diff --git a/drivers/power/supply/ab8500_fg.c b/drivers/power/supply/ab8500_fg.c
index b0919a6a6587..236fd9f9d6f1 100644
--- a/drivers/power/supply/ab8500_fg.c
+++ b/drivers/power/supply/ab8500_fg.c
@@ -2263,7 +2263,13 @@ static int ab8500_fg_init_hw_registers(struct ab8500_fg *di)
{
int ret;
- /* Set VBAT OVV threshold */
+ /*
+ * Set VBAT OVV (overvoltage) threshold to 4.75V (typ) this is what
+ * the hardware supports, nothing else can be configured in hardware.
+ * See this as an "outer limit" where the charger will certainly
+ * shut down. Other (lower) overvoltage levels need to be implemented
+ * in software.
+ */
ret = abx500_mask_and_set_register_interruptible(di->dev,
AB8500_CHARGER,
AB8500_BATT_OVV,
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 03/11] power: supply: ab8500: Integrate thermal zone
2022-01-27 1:12 [PATCH 00/11] AB8500 charging fixes Linus Walleij
2022-01-27 1:12 ` [PATCH 01/11] power: supply: ab8500: Drop BATCTRL thermal mode Linus Walleij
2022-01-27 1:12 ` [PATCH 02/11] power: supply: ab8500: Swap max and overvoltage Linus Walleij
@ 2022-01-27 1:12 ` Linus Walleij
2022-01-27 1:12 ` [PATCH 04/11] power: supply: ab8500_fg: Break loop for measurement Linus Walleij
` (7 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2022-01-27 1:12 UTC (permalink / raw)
To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Linus Walleij
Instead of providing our own homebrewn thermal measurement
code for an NTC and passing tables, we put the NTC thermistor
into the device tree, create a passive thermal zone, and poll
this thermal zone for the temperature.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/power/supply/Kconfig | 2 +
drivers/power/supply/ab8500-bm.h | 33 ---------
drivers/power/supply/ab8500_bmdata.c | 24 -------
drivers/power/supply/ab8500_btemp.c | 103 ++++++---------------------
4 files changed, 24 insertions(+), 138 deletions(-)
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index b366e2fd8e97..6815e5a4c0bd 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -728,6 +728,8 @@ config BATTERY_GAUGE_LTC2941
config AB8500_BM
bool "AB8500 Battery Management Driver"
depends on AB8500_CORE && AB8500_GPADC && (IIO = y) && OF
+ select THERMAL
+ select THERMAL_OF
help
Say Y to include support for AB8500 battery management.
diff --git a/drivers/power/supply/ab8500-bm.h b/drivers/power/supply/ab8500-bm.h
index 3bc6fd9337d2..6efd5174dbce 100644
--- a/drivers/power/supply/ab8500-bm.h
+++ b/drivers/power/supply/ab8500-bm.h
@@ -260,18 +260,6 @@ enum bup_vch_sel {
#define BUS_PP_PRECHG_CURRENT_MASK 0x0E
#define BUS_POWER_PATH_PRECHG_ENA 0x01
-/**
- * struct ab8500_res_to_temp - defines one point in a temp to res curve. To
- * be used in battery packs that combines the identification resistor with a
- * NTC resistor.
- * @temp: battery pack temperature in Celsius
- * @resist: NTC resistor net total resistance
- */
-struct ab8500_res_to_temp {
- int temp;
- int resist;
-};
-
/* Forward declaration */
struct ab8500_fg;
@@ -351,8 +339,6 @@ struct ab8500_maxim_parameters {
* @maint_b_chg_timer_h: charge time in maintenance B state
* @low_high_cur_lvl: charger current in temp low/high state in mA
* @low_high_vol_lvl: charger voltage in temp low/high state in mV'
- * @n_r_t_tbl_elements: number of elements in r_to_t_tbl
- * @r_to_t_tbl: table containing resistance to temp points
*/
struct ab8500_battery_type {
int resis_high;
@@ -365,8 +351,6 @@ struct ab8500_battery_type {
int maint_b_chg_timer_h;
int low_high_cur_lvl;
int low_high_vol_lvl;
- int n_temp_tbl_elements;
- const struct ab8500_res_to_temp *r_to_t_tbl;
};
/**
@@ -449,23 +433,6 @@ struct ab8500_bm_data {
const struct ab8500_fg_parameters *fg_params;
};
-enum {
- NTC_EXTERNAL = 0,
- NTC_INTERNAL,
-};
-
-/**
- * struct res_to_temp - defines one point in a temp to res curve. To
- * be used in battery packs that combines the identification resistor with a
- * NTC resistor.
- * @temp: battery pack temperature in Celsius
- * @resist: NTC resistor net total resistance
- */
-struct res_to_temp {
- int temp;
- int resist;
-};
-
/* Forward declaration */
struct ab8500_fg;
diff --git a/drivers/power/supply/ab8500_bmdata.c b/drivers/power/supply/ab8500_bmdata.c
index 62b63f0437dd..d8fc72be0f0e 100644
--- a/drivers/power/supply/ab8500_bmdata.c
+++ b/drivers/power/supply/ab8500_bmdata.c
@@ -43,28 +43,6 @@ static struct power_supply_battery_ocv_table ocv_cap_tbl[] = {
{ .ocv = 3094000, .capacity = 0},
};
-/*
- * Note that the res_to_temp table must be strictly sorted by falling
- * resistance values to work.
- */
-static const struct ab8500_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. Factory resistance is 300 mOhm and the
@@ -92,8 +70,6 @@ static struct ab8500_battery_type bat_type_thermistor_unknown = {
.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,
};
static const struct ab8500_bm_capacity_levels cap_levels = {
diff --git a/drivers/power/supply/ab8500_btemp.c b/drivers/power/supply/ab8500_btemp.c
index a5ca09124c93..2a6fc151210c 100644
--- a/drivers/power/supply/ab8500_btemp.c
+++ b/drivers/power/supply/ab8500_btemp.c
@@ -26,13 +26,12 @@
#include <linux/mfd/core.h>
#include <linux/mfd/abx500.h>
#include <linux/mfd/abx500/ab8500.h>
+#include <linux/thermal.h>
#include <linux/iio/consumer.h>
#include <linux/fixp-arith.h>
#include "ab8500-bm.h"
-#define VTVOUT_V 1800
-
#define BTEMP_THERMAL_LOW_LIMIT -10
#define BTEMP_THERMAL_MED_LIMIT 0
#define BTEMP_THERMAL_HIGH_LIMIT_52 52
@@ -82,7 +81,7 @@ struct ab8500_btemp_ranges {
* @bat_temp: Dispatched battery temperature in degree Celsius
* @prev_bat_temp Last measured battery temperature in degree Celsius
* @parent: Pointer to the struct ab8500
- * @adc_btemp_ball: ADC channel for the battery ball temperature
+ * @tz: Thermal zone for the battery
* @adc_bat_ctrl: ADC channel for the battery control
* @fg: Pointer to the struct fg
* @bm: Platform specific battery management information
@@ -100,7 +99,7 @@ struct ab8500_btemp {
int bat_temp;
int prev_bat_temp;
struct ab8500 *parent;
- struct iio_channel *btemp_ball;
+ struct thermal_zone_device *tz;
struct iio_channel *bat_ctrl;
struct ab8500_fg *fg;
struct ab8500_bm_data *bm;
@@ -228,76 +227,6 @@ static int ab8500_btemp_get_batctrl_res(struct ab8500_btemp *di)
return res;
}
-/**
- * ab8500_btemp_res_to_temp() - resistance to temperature
- * @di: pointer to the ab8500_btemp structure
- * @tbl: pointer to the resiatance to temperature table
- * @tbl_size: size of the resistance to temperature table
- * @res: resistance to calculate the temperature from
- *
- * This function returns the battery temperature in degrees Celsius
- * based on the NTC resistance.
- */
-static int ab8500_btemp_res_to_temp(struct ab8500_btemp *di,
- const struct ab8500_res_to_temp *tbl, int tbl_size, int res)
-{
- int i;
- /*
- * Calculate the formula for the straight line
- * Simple interpolation if we are within
- * the resistance table limits, extrapolate
- * if resistance is outside the limits.
- */
- if (res > tbl[0].resist)
- i = 0;
- else if (res <= tbl[tbl_size - 1].resist)
- i = tbl_size - 2;
- else {
- i = 0;
- while (!(res <= tbl[i].resist &&
- res > tbl[i + 1].resist))
- i++;
- }
-
- return fixp_linear_interpolate(tbl[i].resist, tbl[i].temp,
- tbl[i + 1].resist, tbl[i + 1].temp,
- res);
-}
-
-/**
- * ab8500_btemp_measure_temp() - measure battery temperature
- * @di: pointer to the ab8500_btemp structure
- *
- * Returns battery temperature (on success) else the previous temperature
- */
-static int ab8500_btemp_measure_temp(struct ab8500_btemp *di)
-{
- int temp, ret;
- static int prev;
- int rntc, vntc;
-
- ret = iio_read_channel_processed(di->btemp_ball, &vntc);
- if (ret < 0) {
- dev_err(di->dev,
- "%s ADC conversion failed,"
- " using previous value\n", __func__);
- return prev;
- }
- /*
- * The PCB NTC is sourced from VTVOUT via a 230kOhm
- * resistor.
- */
- rntc = 230000 * vntc / (VTVOUT_V - vntc);
-
- temp = ab8500_btemp_res_to_temp(di,
- di->bm->bat_type->r_to_t_tbl,
- di->bm->bat_type->n_temp_tbl_elements, rntc);
- prev = temp;
-
- dev_dbg(di->dev, "Battery temperature is %d\n", temp);
- return temp;
-}
-
/**
* ab8500_btemp_id() - Identify the connected battery
* @di: pointer to the ab8500_btemp structure
@@ -347,6 +276,9 @@ static void ab8500_btemp_periodic_work(struct work_struct *work)
int bat_temp;
struct ab8500_btemp *di = container_of(work,
struct ab8500_btemp, btemp_periodic_work.work);
+ /* Assume 25 degrees celsius as start temperature */
+ static int prev = 25;
+ int ret;
if (!di->initialized) {
/* Identify the battery */
@@ -354,7 +286,17 @@ static void ab8500_btemp_periodic_work(struct work_struct *work)
dev_warn(di->dev, "failed to identify the battery\n");
}
- bat_temp = ab8500_btemp_measure_temp(di);
+ /* Failover if a reading is erroneous, use last meausurement */
+ ret = thermal_zone_get_temp(di->tz, &bat_temp);
+ if (ret) {
+ dev_err(di->dev, "error reading temperature\n");
+ bat_temp = prev;
+ } else {
+ /* Convert from millicentigrades to centigrades */
+ bat_temp /= 1000;
+ prev = bat_temp;
+ }
+
/*
* Filter battery temperature.
* Allow direct updates on temperature only if two samples result in
@@ -783,12 +725,11 @@ static int ab8500_btemp_probe(struct platform_device *pdev)
di->dev = dev;
di->parent = dev_get_drvdata(pdev->dev.parent);
- /* Get ADC channels */
- di->btemp_ball = devm_iio_channel_get(dev, "btemp_ball");
- if (IS_ERR(di->btemp_ball)) {
- ret = dev_err_probe(dev, PTR_ERR(di->btemp_ball),
- "failed to get BTEMP BALL ADC channel\n");
- return ret;
+ /* Get thermal zone and ADC */
+ di->tz = thermal_zone_get_zone_by_name("battery-thermal");
+ if (IS_ERR(di->tz)) {
+ return dev_err_probe(dev, PTR_ERR(di->tz),
+ "failed to get battery thermal zone\n");
}
di->bat_ctrl = devm_iio_channel_get(dev, "bat_ctrl");
if (IS_ERR(di->bat_ctrl)) {
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 04/11] power: supply: ab8500_fg: Break loop for measurement
2022-01-27 1:12 [PATCH 00/11] AB8500 charging fixes Linus Walleij
` (2 preceding siblings ...)
2022-01-27 1:12 ` [PATCH 03/11] power: supply: ab8500: Integrate thermal zone Linus Walleij
@ 2022-01-27 1:12 ` Linus Walleij
2022-01-27 1:12 ` [PATCH 05/11] power: supply: ab8500_fg: Break out load compensated voltage Linus Walleij
` (6 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2022-01-27 1:12 UTC (permalink / raw)
To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Linus Walleij
In the Samsung code tree we find that it can happen that this
measurement loop goes on for a long time, and it seems like a
good idea to break it after 70 iterations if it goes on for
too long.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/power/supply/ab8500_fg.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/ab8500_fg.c b/drivers/power/supply/ab8500_fg.c
index 236fd9f9d6f1..29896f09fd17 100644
--- a/drivers/power/supply/ab8500_fg.c
+++ b/drivers/power/supply/ab8500_fg.c
@@ -45,6 +45,7 @@
#define SEC_TO_SAMPLE(S) (S * 4)
#define NBR_AVG_SAMPLES 20
+#define WAIT_FOR_INST_CURRENT_MAX 70
#define LOW_BAT_CHECK_INTERVAL (HZ / 16) /* 62.5 ms */
@@ -926,10 +927,18 @@ static int ab8500_fg_load_comp_volt_to_capacity(struct ab8500_fg *di)
vbat_uv += ab8500_fg_bat_voltage(di);
i++;
usleep_range(5000, 6000);
- } while (!ab8500_fg_inst_curr_done(di));
+ } while (!ab8500_fg_inst_curr_done(di) &&
+ i <= WAIT_FOR_INST_CURRENT_MAX);
+
+ if (i > WAIT_FOR_INST_CURRENT_MAX) {
+ dev_err(di->dev,
+ "TIMEOUT: return capacity based on uncompensated measurement of VBAT\n");
+ goto calc_cap;
+ }
ab8500_fg_inst_curr_finalize(di, &di->inst_curr_ua);
+calc_cap:
di->vbat_uv = vbat_uv / i;
res = ab8500_fg_battery_resistance(di);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 05/11] power: supply: ab8500_fg: Break out load compensated voltage
2022-01-27 1:12 [PATCH 00/11] AB8500 charging fixes Linus Walleij
` (3 preceding siblings ...)
2022-01-27 1:12 ` [PATCH 04/11] power: supply: ab8500_fg: Break loop for measurement Linus Walleij
@ 2022-01-27 1:12 ` Linus Walleij
2022-01-27 22:31 ` kernel test robot
2022-01-27 1:12 ` [PATCH 06/11] power: supply: ab8500_fg: Safeguard " Linus Walleij
` (5 subsequent siblings)
10 siblings, 1 reply; 13+ messages in thread
From: Linus Walleij @ 2022-01-27 1:12 UTC (permalink / raw)
To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Linus Walleij
Break out the part of the function providing the load compensated
capacity that provides the load compensated voltage and use
that to get the load compensated capacity.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/power/supply/ab8500_fg.c | 50 ++++++++++++++++++++------------
1 file changed, 32 insertions(+), 18 deletions(-)
diff --git a/drivers/power/supply/ab8500_fg.c b/drivers/power/supply/ab8500_fg.c
index 29896f09fd17..8c907900a56d 100644
--- a/drivers/power/supply/ab8500_fg.c
+++ b/drivers/power/supply/ab8500_fg.c
@@ -909,18 +909,20 @@ static int ab8500_fg_battery_resistance(struct ab8500_fg *di)
}
/**
- * ab8500_fg_load_comp_volt_to_capacity() - Load compensated voltage based capacity
+ * ab8500_load_comp_fg_bat_voltage() - get load compensated battery voltage
* @di: pointer to the ab8500_fg structure
*
- * Returns battery capacity based on battery voltage that is load compensated
- * for the voltage drop
+ * Returns compensated battery voltage (on success) else error code.
+ * If always is specified, we always return a voltage but it may be
+ * uncompensated.
*/
-static int ab8500_fg_load_comp_volt_to_capacity(struct ab8500_fg *di)
+static int ab8500_load_comp_fg_bat_voltage(struct ab8500_fg *di)
{
- int vbat_comp_uv, res;
int i = 0;
int vbat_uv = 0;
+ int rcomp;
+ /* Average the instant current to get a stable current measurement */
ab8500_fg_inst_curr_start(di);
do {
@@ -932,25 +934,37 @@ static int ab8500_fg_load_comp_volt_to_capacity(struct ab8500_fg *di)
if (i > WAIT_FOR_INST_CURRENT_MAX) {
dev_err(di->dev,
- "TIMEOUT: return capacity based on uncompensated measurement of VBAT\n");
- goto calc_cap;
+ "TIMEOUT: return uncompensated measurement of VBAT\n");
+ di->vbat_uv = vbat_uv / i;
+ return di->vbat_uv;
}
ab8500_fg_inst_curr_finalize(di, &di->inst_curr_ua);
-calc_cap:
- di->vbat_uv = vbat_uv / i;
- res = ab8500_fg_battery_resistance(di);
+ vbat_uv = vbat_uv / i;
- /*
- * Use Ohms law to get the load compensated voltage.
- * Divide by 1000 to get from milliohms to ohms.
- */
- vbat_comp_uv = di->vbat_uv - (di->inst_curr_ua * res) / 1000;
+ /* Next we apply voltage compensation from internal resistance */
+ rcomp = ab8500_fg_battery_resistance(di, vbat_uv);
+ vbat_uv = vbat_uv - (di->inst_curr_ua * rcomp) / 1000;
+
+ /* Always keep this state at latest measurement */
+ di->vbat_uv = vbat_uv;
+
+ return vbat_uv;
+}
+
+/**
+ * ab8500_fg_load_comp_volt_to_capacity() - Load compensated voltage based capacity
+ * @di: pointer to the ab8500_fg structure
+ *
+ * Returns battery capacity based on battery voltage that is load compensated
+ * for the voltage drop
+ */
+static int ab8500_fg_load_comp_volt_to_capacity(struct ab8500_fg *di)
+{
+ int vbat_comp_uv;
- dev_dbg(di->dev, "%s Measured Vbat: %d uV,Compensated Vbat %d uV, "
- "R: %d mOhm, Current: %d uA Vbat Samples: %d\n",
- __func__, di->vbat_uv, vbat_comp_uv, res, di->inst_curr_ua, i);
+ vbat_comp_uv = ab8500_load_comp_fg_bat_voltage(di);
return ab8500_fg_volt_to_capacity(di, vbat_comp_uv);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 05/11] power: supply: ab8500_fg: Break out load compensated voltage
2022-01-27 1:12 ` [PATCH 05/11] power: supply: ab8500_fg: Break out load compensated voltage Linus Walleij
@ 2022-01-27 22:31 ` kernel test robot
0 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2022-01-27 22:31 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4199 bytes --]
Hi Linus,
I love your patch! Yet something to improve:
[auto build test ERROR on sre-power-supply/for-next]
[also build test ERROR on v5.17-rc1 next-20220127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Linus-Walleij/AB8500-charging-fixes/20220127-091555
base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: arm-allyesconfig (https://download.01.org/0day-ci/archive/20220128/202201280613.1yVNjErX-lkp(a)intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/b950e6ce295a6cfa6175ece7b11f9e0e904e35ee
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Linus-Walleij/AB8500-charging-fixes/20220127-091555
git checkout b950e6ce295a6cfa6175ece7b11f9e0e904e35ee
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/power/supply/ab8500_fg.c: In function 'ab8500_load_comp_fg_bat_voltage':
>> drivers/power/supply/ab8500_fg.c:947:17: error: too many arguments to function 'ab8500_fg_battery_resistance'
947 | rcomp = ab8500_fg_battery_resistance(di, vbat_uv);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/power/supply/ab8500_fg.c:883:12: note: declared here
883 | static int ab8500_fg_battery_resistance(struct ab8500_fg *di)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/power/supply/ab8500_fg.c: In function 'ab8500_fg_get_ext_psy_data':
drivers/power/supply/ab8500_fg.c:2237:75: warning: variable 'b' set but not used [-Wunused-but-set-variable]
2237 | const struct ab8500_battery_type *b;
| ^
vim +/ab8500_fg_battery_resistance +947 drivers/power/supply/ab8500_fg.c
910
911 /**
912 * ab8500_load_comp_fg_bat_voltage() - get load compensated battery voltage
913 * @di: pointer to the ab8500_fg structure
914 *
915 * Returns compensated battery voltage (on success) else error code.
916 * If always is specified, we always return a voltage but it may be
917 * uncompensated.
918 */
919 static int ab8500_load_comp_fg_bat_voltage(struct ab8500_fg *di)
920 {
921 int i = 0;
922 int vbat_uv = 0;
923 int rcomp;
924
925 /* Average the instant current to get a stable current measurement */
926 ab8500_fg_inst_curr_start(di);
927
928 do {
929 vbat_uv += ab8500_fg_bat_voltage(di);
930 i++;
931 usleep_range(5000, 6000);
932 } while (!ab8500_fg_inst_curr_done(di) &&
933 i <= WAIT_FOR_INST_CURRENT_MAX);
934
935 if (i > WAIT_FOR_INST_CURRENT_MAX) {
936 dev_err(di->dev,
937 "TIMEOUT: return uncompensated measurement of VBAT\n");
938 di->vbat_uv = vbat_uv / i;
939 return di->vbat_uv;
940 }
941
942 ab8500_fg_inst_curr_finalize(di, &di->inst_curr_ua);
943
944 vbat_uv = vbat_uv / i;
945
946 /* Next we apply voltage compensation from internal resistance */
> 947 rcomp = ab8500_fg_battery_resistance(di, vbat_uv);
948 vbat_uv = vbat_uv - (di->inst_curr_ua * rcomp) / 1000;
949
950 /* Always keep this state at latest measurement */
951 di->vbat_uv = vbat_uv;
952
953 return vbat_uv;
954 }
955
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 06/11] power: supply: ab8500_fg: Safeguard compensated voltage
2022-01-27 1:12 [PATCH 00/11] AB8500 charging fixes Linus Walleij
` (4 preceding siblings ...)
2022-01-27 1:12 ` [PATCH 05/11] power: supply: ab8500_fg: Break out load compensated voltage Linus Walleij
@ 2022-01-27 1:12 ` Linus Walleij
2022-01-27 1:12 ` [PATCH 07/11] power: supply: ab8500_fg: Drop useless parameter Linus Walleij
` (4 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2022-01-27 1:12 UTC (permalink / raw)
To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Linus Walleij
In some cases when the platform is dissapating more than
500mA the voltage measurements and compensation will become
instable. Add a parameter to bail out of the voltage
measurement if this happens.
This code was found in a Samsung vendor tree.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/power/supply/ab8500_fg.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/power/supply/ab8500_fg.c b/drivers/power/supply/ab8500_fg.c
index 8c907900a56d..323f6bacefd8 100644
--- a/drivers/power/supply/ab8500_fg.c
+++ b/drivers/power/supply/ab8500_fg.c
@@ -46,6 +46,8 @@
#define NBR_AVG_SAMPLES 20
#define WAIT_FOR_INST_CURRENT_MAX 70
+/* Currents higher than -500mA (dissipating) will make compensation unstable */
+#define IGNORE_VBAT_HIGHCUR -500000
#define LOW_BAT_CHECK_INTERVAL (HZ / 16) /* 62.5 ms */
@@ -911,12 +913,13 @@ static int ab8500_fg_battery_resistance(struct ab8500_fg *di)
/**
* ab8500_load_comp_fg_bat_voltage() - get load compensated battery voltage
* @di: pointer to the ab8500_fg structure
+ * @always: always return a voltage, also uncompensated
*
* Returns compensated battery voltage (on success) else error code.
* If always is specified, we always return a voltage but it may be
* uncompensated.
*/
-static int ab8500_load_comp_fg_bat_voltage(struct ab8500_fg *di)
+static int ab8500_load_comp_fg_bat_voltage(struct ab8500_fg *di, bool always)
{
int i = 0;
int vbat_uv = 0;
@@ -941,6 +944,14 @@ static int ab8500_load_comp_fg_bat_voltage(struct ab8500_fg *di)
ab8500_fg_inst_curr_finalize(di, &di->inst_curr_ua);
+ /*
+ * If there is too high current dissipation, the compensation cannot be
+ * trusted so return an error unless we must return something here, as
+ * enforced by the "always" parameter.
+ */
+ if (!always && di->inst_curr_ua < IGNORE_VBAT_HIGHCUR)
+ return -EINVAL;
+
vbat_uv = vbat_uv / i;
/* Next we apply voltage compensation from internal resistance */
@@ -964,7 +975,7 @@ static int ab8500_fg_load_comp_volt_to_capacity(struct ab8500_fg *di)
{
int vbat_comp_uv;
- vbat_comp_uv = ab8500_load_comp_fg_bat_voltage(di);
+ vbat_comp_uv = ab8500_load_comp_fg_bat_voltage(di, true);
return ab8500_fg_volt_to_capacity(di, vbat_comp_uv);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 07/11] power: supply: ab8500_fg: Drop useless parameter
2022-01-27 1:12 [PATCH 00/11] AB8500 charging fixes Linus Walleij
` (5 preceding siblings ...)
2022-01-27 1:12 ` [PATCH 06/11] power: supply: ab8500_fg: Safeguard " Linus Walleij
@ 2022-01-27 1:12 ` Linus Walleij
2022-01-27 1:12 ` [PATCH 08/11] power: supply: ab8500_chargalg: Drop charging step Linus Walleij
` (3 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2022-01-27 1:12 UTC (permalink / raw)
To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Linus Walleij
All calls to ab8500_fg_calc_cap_discharge_voltage() require
compensation and pass true as the second argument so just drop
this argument.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/power/supply/ab8500_fg.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/power/supply/ab8500_fg.c b/drivers/power/supply/ab8500_fg.c
index 323f6bacefd8..fa61ecebbce3 100644
--- a/drivers/power/supply/ab8500_fg.c
+++ b/drivers/power/supply/ab8500_fg.c
@@ -1073,20 +1073,16 @@ static int ab8500_fg_calc_cap_charging(struct ab8500_fg *di)
/**
* ab8500_fg_calc_cap_discharge_voltage() - Capacity in discharge with voltage
* @di: pointer to the ab8500_fg structure
- * @comp: if voltage should be load compensated before capacity calc
*
- * Return the capacity in mAh based on the battery voltage. The voltage can
- * either be load compensated or not. This value is added to the filter and a
- * new mean value is calculated and returned.
+ * Return the capacity in mAh based on the load compensated battery voltage.
+ * This value is added to the filter and a new mean value is calculated and
+ * returned.
*/
-static int ab8500_fg_calc_cap_discharge_voltage(struct ab8500_fg *di, bool comp)
+static int ab8500_fg_calc_cap_discharge_voltage(struct ab8500_fg *di)
{
int permille, mah;
- if (comp)
- permille = ab8500_fg_load_comp_volt_to_capacity(di);
- else
- permille = ab8500_fg_uncomp_volt_to_capacity(di);
+ permille = ab8500_fg_load_comp_volt_to_capacity(di);
mah = ab8500_fg_convert_permille_to_mah(di, permille);
@@ -1563,7 +1559,7 @@ static void ab8500_fg_algorithm_discharging(struct ab8500_fg *di)
/* Discard the first [x] seconds */
if (di->init_cnt > di->bm->fg_params->init_discard_time) {
- ab8500_fg_calc_cap_discharge_voltage(di, true);
+ ab8500_fg_calc_cap_discharge_voltage(di);
ab8500_fg_check_capacity_limits(di, true);
}
@@ -1646,7 +1642,7 @@ static void ab8500_fg_algorithm_discharging(struct ab8500_fg *di)
break;
}
- ab8500_fg_calc_cap_discharge_voltage(di, true);
+ ab8500_fg_calc_cap_discharge_voltage(di);
} else {
mutex_lock(&di->cc_lock);
if (!di->flags.conv_done) {
@@ -1680,7 +1676,7 @@ static void ab8500_fg_algorithm_discharging(struct ab8500_fg *di)
break;
case AB8500_FG_DISCHARGE_WAKEUP:
- ab8500_fg_calc_cap_discharge_voltage(di, true);
+ ab8500_fg_calc_cap_discharge_voltage(di);
di->fg_samples = SEC_TO_SAMPLE(
di->bm->fg_params->accu_high_curr);
@@ -1799,7 +1795,7 @@ static void ab8500_fg_periodic_work(struct work_struct *work)
if (di->init_capacity) {
/* Get an initial capacity calculation */
- ab8500_fg_calc_cap_discharge_voltage(di, true);
+ ab8500_fg_calc_cap_discharge_voltage(di);
ab8500_fg_check_capacity_limits(di, true);
di->init_capacity = false;
@@ -2422,7 +2418,7 @@ static void ab8500_fg_reinit_work(struct work_struct *work)
if (!di->flags.calibrate) {
dev_dbg(di->dev, "Resetting FG state machine to init.\n");
ab8500_fg_clear_cap_samples(di);
- ab8500_fg_calc_cap_discharge_voltage(di, true);
+ ab8500_fg_calc_cap_discharge_voltage(di);
ab8500_fg_charge_state_to(di, AB8500_FG_CHARGE_INIT);
ab8500_fg_discharge_state_to(di, AB8500_FG_DISCHARGE_INIT);
queue_delayed_work(di->fg_wq, &di->fg_periodic_work, 0);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 08/11] power: supply: ab8500_chargalg: Drop charging step
2022-01-27 1:12 [PATCH 00/11] AB8500 charging fixes Linus Walleij
` (6 preceding siblings ...)
2022-01-27 1:12 ` [PATCH 07/11] power: supply: ab8500_fg: Drop useless parameter Linus Walleij
@ 2022-01-27 1:12 ` Linus Walleij
2022-01-27 1:12 ` [PATCH 09/11] power: supply: ab8500_chargalg: Drop enable/disable sysfs Linus Walleij
` (2 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2022-01-27 1:12 UTC (permalink / raw)
To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Linus Walleij
There is a sysfs ABI to change the "charging step" of the
charger i.e. limit how much we charge from userspace.
Since we don't have any userspace for this code, this sits
unused and it is not used on production products either.
Drop this code.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/power/supply/ab8500_chargalg.c | 105 ++-----------------------
1 file changed, 6 insertions(+), 99 deletions(-)
diff --git a/drivers/power/supply/ab8500_chargalg.c b/drivers/power/supply/ab8500_chargalg.c
index bcf85ae6828e..9f9a84ad2da2 100644
--- a/drivers/power/supply/ab8500_chargalg.c
+++ b/drivers/power/supply/ab8500_chargalg.c
@@ -46,9 +46,6 @@
/* Five minutes expressed in seconds */
#define FIVE_MINUTES_IN_SECONDS 300
-#define CHARGALG_CURR_STEP_LOW_UA 0
-#define CHARGALG_CURR_STEP_HIGH_UA 100000
-
/*
* This is the battery capacity limit that will trigger a new
* full charging cycle in the case where maintenance charging
@@ -86,11 +83,6 @@ struct ab8500_chargalg_suspension_status {
bool usb_suspended;
};
-struct ab8500_chargalg_current_step_status {
- bool curr_step_change;
- int curr_step_ua;
-};
-
struct ab8500_chargalg_battery_data {
int temp;
int volt_uv;
@@ -186,8 +178,6 @@ struct ab8500_chargalg_events {
* struct ab8500_charge_curr_maximization - Charger maximization parameters
* @original_iset_ua: the non optimized/maximised charger current
* @current_iset_ua: the charging current used at this moment
- * @test_delta_i_ua: the delta between the current we want to charge and the
- current that is really going into the battery
* @condition_cnt: number of iterations needed before a new charger current
is set
* @max_current_ua: maximum charger current
@@ -200,7 +190,6 @@ struct ab8500_chargalg_events {
struct ab8500_charge_curr_maximization {
int original_iset_ua;
int current_iset_ua;
- int test_delta_i_ua;
int condition_cnt;
int max_current_ua;
int wait_cnt;
@@ -229,7 +218,6 @@ enum maxim_ret {
* @batt_data: data of the battery
* @susp_status: current charger suspension status
* @bm: Platform specific battery management information
- * @curr_status: Current step status for over-current protection
* @parent: pointer to the struct ab8500
* @chargalg_psy: structure that holds the battery properties exposed by
* the charging algorithm
@@ -255,7 +243,6 @@ struct ab8500_chargalg {
struct ab8500_chargalg_battery_data batt_data;
struct ab8500_chargalg_suspension_status susp_status;
struct ab8500 *parent;
- struct ab8500_chargalg_current_step_status curr_status;
struct ab8500_bm_data *bm;
struct power_supply *chargalg_psy;
struct ux500_charger *ac_chg;
@@ -420,22 +407,6 @@ static int ab8500_chargalg_check_charger_connection(struct ab8500_chargalg *di)
return di->chg_info.conn_chg;
}
-/**
- * ab8500_chargalg_check_current_step_status() - Check charging current
- * step status.
- * @di: pointer to the ab8500_chargalg structure
- *
- * This function will check if there is a change in the charging current step
- * and change charge state accordingly.
- */
-static void ab8500_chargalg_check_current_step_status
- (struct ab8500_chargalg *di)
-{
- if (di->curr_status.curr_step_change)
- ab8500_chargalg_state_to(di, STATE_NORMAL_INIT);
- di->curr_status.curr_step_change = false;
-}
-
/**
* ab8500_chargalg_start_safety_timer() - Start charging safety timer
* @di: pointer to the ab8500_chargalg structure
@@ -831,7 +802,6 @@ static void init_maxim_chg_curr(struct ab8500_chargalg *di)
di->ccm.original_iset_ua = bi->constant_charge_current_max_ua;
di->ccm.current_iset_ua = bi->constant_charge_current_max_ua;
- di->ccm.test_delta_i_ua = di->bm->maxi->charger_curr_step_ua;
di->ccm.max_current_ua = di->bm->maxi->chg_curr_ua;
di->ccm.condition_cnt = di->bm->maxi->wait_cycles;
di->ccm.level = 0;
@@ -862,8 +832,7 @@ static enum maxim_ret ab8500_chargalg_chg_curr_maxim(struct ab8500_chargalg *di)
dev_dbg(di->dev, "lowering current\n");
di->ccm.wait_cnt++;
di->ccm.condition_cnt = di->bm->maxi->wait_cycles;
- di->ccm.max_current_ua =
- di->ccm.current_iset_ua - di->ccm.test_delta_i_ua;
+ di->ccm.max_current_ua = di->ccm.current_iset_ua;
di->ccm.current_iset_ua = di->ccm.max_current_ua;
di->ccm.level--;
return MAXIM_RET_CHANGE;
@@ -893,29 +862,8 @@ static enum maxim_ret ab8500_chargalg_chg_curr_maxim(struct ab8500_chargalg *di)
return MAXIM_RET_IBAT_TOO_HIGH;
}
- if (delta_i_ua > di->ccm.test_delta_i_ua &&
- (di->ccm.current_iset_ua + di->ccm.test_delta_i_ua) <
- di->ccm.max_current_ua) {
- if (di->ccm.condition_cnt-- == 0) {
- /* Increse the iset with cco.test_delta_i */
- di->ccm.condition_cnt = di->bm->maxi->wait_cycles;
- di->ccm.current_iset_ua += di->ccm.test_delta_i_ua;
- di->ccm.level++;
- dev_dbg(di->dev, " Maximization needed, increase"
- " with %d uA to %duA (Optimal ibat: %d uA)"
- " Level %d\n",
- di->ccm.test_delta_i_ua,
- di->ccm.current_iset_ua,
- di->ccm.original_iset_ua,
- di->ccm.level);
- return MAXIM_RET_CHANGE;
- } else {
- return MAXIM_RET_NOACTION;
- }
- } else {
- di->ccm.condition_cnt = di->bm->maxi->wait_cycles;
- return MAXIM_RET_NOACTION;
- }
+ di->ccm.condition_cnt = di->bm->maxi->wait_cycles;
+ return MAXIM_RET_NOACTION;
}
static void handle_maxim_chg_curr(struct ab8500_chargalg *di)
@@ -1302,7 +1250,6 @@ static void ab8500_chargalg_algorithm(struct ab8500_chargalg *di)
struct power_supply_battery_info *bi = di->bm->bi;
int charger_status;
int ret;
- int curr_step_lvl_ua;
/* Collect data from all power_supply class devices */
class_for_each_device(power_supply_class, NULL,
@@ -1313,7 +1260,6 @@ static void ab8500_chargalg_algorithm(struct ab8500_chargalg *di)
ab8500_chargalg_check_charger_voltage(di);
charger_status = ab8500_chargalg_check_charger_connection(di);
- ab8500_chargalg_check_current_step_status(di);
if (is_ab8500(di->parent)) {
ret = ab8500_chargalg_check_charger_enable(di);
@@ -1511,15 +1457,13 @@ static void ab8500_chargalg_algorithm(struct ab8500_chargalg *di)
break;
case STATE_NORMAL_INIT:
- if (di->curr_status.curr_step_ua == CHARGALG_CURR_STEP_LOW_UA)
+ if (bi->constant_charge_current_max_ua == 0)
+ /* "charging" with 0 uA */
ab8500_chargalg_stop_charging(di);
else {
- curr_step_lvl_ua = bi->constant_charge_current_max_ua
- * di->curr_status.curr_step_ua
- / CHARGALG_CURR_STEP_HIGH_UA;
ab8500_chargalg_start_charging(di,
bi->constant_charge_voltage_max_uv,
- curr_step_lvl_ua);
+ bi->constant_charge_current_max_ua);
}
ab8500_chargalg_state_to(di, STATE_NORMAL);
@@ -1742,37 +1686,6 @@ static int ab8500_chargalg_get_property(struct power_supply *psy,
/* Exposure to the sysfs interface */
-static ssize_t ab8500_chargalg_curr_step_show(struct ab8500_chargalg *di,
- char *buf)
-{
- return sprintf(buf, "%d\n", di->curr_status.curr_step_ua);
-}
-
-static ssize_t ab8500_chargalg_curr_step_store(struct ab8500_chargalg *di,
- const char *buf, size_t length)
-{
- long param;
- int ret;
-
- ret = kstrtol(buf, 10, ¶m);
- if (ret < 0)
- return ret;
-
- di->curr_status.curr_step_ua = param;
- if (di->curr_status.curr_step_ua >= CHARGALG_CURR_STEP_LOW_UA &&
- di->curr_status.curr_step_ua <= CHARGALG_CURR_STEP_HIGH_UA) {
- di->curr_status.curr_step_change = true;
- queue_work(di->chargalg_wq, &di->chargalg_work);
- } else
- dev_info(di->dev, "Wrong current step\n"
- "Enter 0. Disable AC/USB Charging\n"
- "1--100. Set AC/USB charging current step\n"
- "100. Enable AC/USB Charging\n");
-
- return strlen(buf);
-}
-
-
static ssize_t ab8500_chargalg_en_show(struct ab8500_chargalg *di,
char *buf)
{
@@ -1832,10 +1745,6 @@ static struct ab8500_chargalg_sysfs_entry ab8500_chargalg_en_charger =
__ATTR(chargalg, 0644, ab8500_chargalg_en_show,
ab8500_chargalg_en_store);
-static struct ab8500_chargalg_sysfs_entry ab8500_chargalg_curr_step =
- __ATTR(chargalg_curr_step, 0644, ab8500_chargalg_curr_step_show,
- ab8500_chargalg_curr_step_store);
-
static ssize_t ab8500_chargalg_sysfs_show(struct kobject *kobj,
struct attribute *attr, char *buf)
{
@@ -1868,7 +1777,6 @@ static ssize_t ab8500_chargalg_sysfs_charger(struct kobject *kobj,
static struct attribute *ab8500_chargalg_chg[] = {
&ab8500_chargalg_en_charger.attr,
- &ab8500_chargalg_curr_step.attr,
NULL,
};
@@ -2057,7 +1965,6 @@ static int ab8500_chargalg_probe(struct platform_device *pdev)
dev_err(di->dev, "failed to create sysfs entry\n");
return ret;
}
- di->curr_status.curr_step_ua = CHARGALG_CURR_STEP_HIGH_UA;
dev_info(di->dev, "probe success\n");
return component_add(dev, &ab8500_chargalg_component_ops);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 09/11] power: supply: ab8500_chargalg: Drop enable/disable sysfs
2022-01-27 1:12 [PATCH 00/11] AB8500 charging fixes Linus Walleij
` (7 preceding siblings ...)
2022-01-27 1:12 ` [PATCH 08/11] power: supply: ab8500_chargalg: Drop charging step Linus Walleij
@ 2022-01-27 1:12 ` Linus Walleij
2022-01-27 1:12 ` [PATCH 10/11] power: supply: ab8500_charger: Restrict ADC retrieveal Linus Walleij
2022-01-27 1:12 ` [PATCH 11/11] power: supply: ab8500_charger: Fix VBAT interval check Linus Walleij
10 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2022-01-27 1:12 UTC (permalink / raw)
To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Linus Walleij
There is a sysfs ABI to enable/disable charging of different
types (AC/USB).
Since we don't have any userspace for this code, this sits
unused and it is not used on production products either.
Drop this code.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/power/supply/ab8500_chargalg.c | 211 +------------------------
1 file changed, 6 insertions(+), 205 deletions(-)
diff --git a/drivers/power/supply/ab8500_chargalg.c b/drivers/power/supply/ab8500_chargalg.c
index 9f9a84ad2da2..b5a3096e78a1 100644
--- a/drivers/power/supply/ab8500_chargalg.c
+++ b/drivers/power/supply/ab8500_chargalg.c
@@ -77,12 +77,6 @@ struct ab8500_chargalg_charger_info {
int ac_iset_ua;
};
-struct ab8500_chargalg_suspension_status {
- bool suspended_change;
- bool ac_suspended;
- bool usb_suspended;
-};
-
struct ab8500_chargalg_battery_data {
int temp;
int volt_uv;
@@ -110,8 +104,6 @@ enum ab8500_chargalg_states {
STATE_TEMP_UNDEROVER,
STATE_TEMP_LOWHIGH_INIT,
STATE_TEMP_LOWHIGH,
- STATE_SUSPENDED_INIT,
- STATE_SUSPENDED,
STATE_OVV_PROTECT_INIT,
STATE_OVV_PROTECT,
STATE_SAFETY_TIMER_EXPIRED_INIT,
@@ -141,8 +133,6 @@ static const char * const states[] = {
"TEMP_UNDEROVER",
"TEMP_LOWHIGH_INIT",
"TEMP_LOWHIGH",
- "SUSPENDED_INIT",
- "SUSPENDED",
"OVV_PROTECT_INIT",
"OVV_PROTECT",
"SAFETY_TIMER_EXPIRED_INIT",
@@ -216,7 +206,6 @@ enum maxim_ret {
* @ccm charging current maximization parameters
* @chg_info: information about connected charger types
* @batt_data: data of the battery
- * @susp_status: current charger suspension status
* @bm: Platform specific battery management information
* @parent: pointer to the struct ab8500
* @chargalg_psy: structure that holds the battery properties exposed by
@@ -241,7 +230,6 @@ struct ab8500_chargalg {
struct ab8500_charge_curr_maximization ccm;
struct ab8500_chargalg_charger_info chg_info;
struct ab8500_chargalg_battery_data batt_data;
- struct ab8500_chargalg_suspension_status susp_status;
struct ab8500 *parent;
struct ab8500_bm_data *bm;
struct power_supply *chargalg_psy;
@@ -372,37 +360,24 @@ static int ab8500_chargalg_check_charger_enable(struct ab8500_chargalg *di)
*/
static int ab8500_chargalg_check_charger_connection(struct ab8500_chargalg *di)
{
- if (di->chg_info.conn_chg != di->chg_info.prev_conn_chg ||
- di->susp_status.suspended_change) {
- /*
- * Charger state changed or suspension
- * has changed since last update
- */
- if ((di->chg_info.conn_chg & AC_CHG) &&
- !di->susp_status.ac_suspended) {
- dev_dbg(di->dev, "Charging source is AC\n");
+ if (di->chg_info.conn_chg != di->chg_info.prev_conn_chg) {
+ /* Charger state changed since last update */
+ if (di->chg_info.conn_chg & AC_CHG) {
+ dev_info(di->dev, "Charging source is AC\n");
if (di->chg_info.charger_type != AC_CHG) {
di->chg_info.charger_type = AC_CHG;
ab8500_chargalg_state_to(di, STATE_NORMAL_INIT);
}
- } else if ((di->chg_info.conn_chg & USB_CHG) &&
- !di->susp_status.usb_suspended) {
- dev_dbg(di->dev, "Charging source is USB\n");
+ } else if (di->chg_info.conn_chg & USB_CHG) {
+ dev_info(di->dev, "Charging source is USB\n");
di->chg_info.charger_type = USB_CHG;
ab8500_chargalg_state_to(di, STATE_NORMAL_INIT);
- } else if (di->chg_info.conn_chg &&
- (di->susp_status.ac_suspended ||
- di->susp_status.usb_suspended)) {
- dev_dbg(di->dev, "Charging is suspended\n");
- di->chg_info.charger_type = NO_CHG;
- ab8500_chargalg_state_to(di, STATE_SUSPENDED_INIT);
} else {
dev_dbg(di->dev, "Charging source is OFF\n");
di->chg_info.charger_type = NO_CHG;
ab8500_chargalg_state_to(di, STATE_HANDHELD_INIT);
}
di->chg_info.prev_conn_chg = di->chg_info.conn_chg;
- di->susp_status.suspended_change = false;
}
return di->chg_info.conn_chg;
}
@@ -1281,12 +1256,6 @@ static void ab8500_chargalg_algorithm(struct ab8500_chargalg *di)
}
}
- /* If suspended, we should not continue checking the flags */
- else if (di->charge_state == STATE_SUSPENDED_INIT ||
- di->charge_state == STATE_SUSPENDED) {
- /* We don't do anything here, just don,t continue */
- }
-
/* Safety timer expiration */
else if (di->events.safety_timer_expired) {
if (di->charge_state != STATE_SAFETY_TIMER_EXPIRED)
@@ -1384,23 +1353,6 @@ static void ab8500_chargalg_algorithm(struct ab8500_chargalg *di)
case STATE_HANDHELD:
break;
- case STATE_SUSPENDED_INIT:
- if (di->susp_status.ac_suspended)
- ab8500_chargalg_ac_en(di, false, 0, 0);
- if (di->susp_status.usb_suspended)
- ab8500_chargalg_usb_en(di, false, 0, 0);
- ab8500_chargalg_stop_safety_timer(di);
- ab8500_chargalg_stop_maintenance_timer(di);
- di->charge_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
- di->maintenance_chg = false;
- ab8500_chargalg_state_to(di, STATE_SUSPENDED);
- power_supply_changed(di->chargalg_psy);
- fallthrough;
-
- case STATE_SUSPENDED:
- /* CHARGING is suspended */
- break;
-
case STATE_BATT_REMOVED_INIT:
ab8500_chargalg_stop_charging(di);
ab8500_chargalg_state_to(di, STATE_BATT_REMOVED);
@@ -1684,144 +1636,6 @@ static int ab8500_chargalg_get_property(struct power_supply *psy,
return 0;
}
-/* Exposure to the sysfs interface */
-
-static ssize_t ab8500_chargalg_en_show(struct ab8500_chargalg *di,
- char *buf)
-{
- return sprintf(buf, "%d\n",
- di->susp_status.ac_suspended &&
- di->susp_status.usb_suspended);
-}
-
-static ssize_t ab8500_chargalg_en_store(struct ab8500_chargalg *di,
- const char *buf, size_t length)
-{
- long param;
- int ac_usb;
- int ret;
-
- ret = kstrtol(buf, 10, ¶m);
- if (ret < 0)
- return ret;
-
- ac_usb = param;
- switch (ac_usb) {
- case 0:
- /* Disable charging */
- di->susp_status.ac_suspended = true;
- di->susp_status.usb_suspended = true;
- di->susp_status.suspended_change = true;
- /* Trigger a state change */
- queue_work(di->chargalg_wq,
- &di->chargalg_work);
- break;
- case 1:
- /* Enable AC Charging */
- di->susp_status.ac_suspended = false;
- di->susp_status.suspended_change = true;
- /* Trigger a state change */
- queue_work(di->chargalg_wq,
- &di->chargalg_work);
- break;
- case 2:
- /* Enable USB charging */
- di->susp_status.usb_suspended = false;
- di->susp_status.suspended_change = true;
- /* Trigger a state change */
- queue_work(di->chargalg_wq,
- &di->chargalg_work);
- break;
- default:
- dev_info(di->dev, "Wrong input\n"
- "Enter 0. Disable AC/USB Charging\n"
- "1. Enable AC charging\n"
- "2. Enable USB Charging\n");
- }
- return strlen(buf);
-}
-
-static struct ab8500_chargalg_sysfs_entry ab8500_chargalg_en_charger =
- __ATTR(chargalg, 0644, ab8500_chargalg_en_show,
- ab8500_chargalg_en_store);
-
-static ssize_t ab8500_chargalg_sysfs_show(struct kobject *kobj,
- struct attribute *attr, char *buf)
-{
- struct ab8500_chargalg_sysfs_entry *entry = container_of(attr,
- struct ab8500_chargalg_sysfs_entry, attr);
-
- struct ab8500_chargalg *di = container_of(kobj,
- struct ab8500_chargalg, chargalg_kobject);
-
- if (!entry->show)
- return -EIO;
-
- return entry->show(di, buf);
-}
-
-static ssize_t ab8500_chargalg_sysfs_charger(struct kobject *kobj,
- struct attribute *attr, const char *buf, size_t length)
-{
- struct ab8500_chargalg_sysfs_entry *entry = container_of(attr,
- struct ab8500_chargalg_sysfs_entry, attr);
-
- struct ab8500_chargalg *di = container_of(kobj,
- struct ab8500_chargalg, chargalg_kobject);
-
- if (!entry->store)
- return -EIO;
-
- return entry->store(di, buf, length);
-}
-
-static struct attribute *ab8500_chargalg_chg[] = {
- &ab8500_chargalg_en_charger.attr,
- NULL,
-};
-
-static const struct sysfs_ops ab8500_chargalg_sysfs_ops = {
- .show = ab8500_chargalg_sysfs_show,
- .store = ab8500_chargalg_sysfs_charger,
-};
-
-static struct kobj_type ab8500_chargalg_ktype = {
- .sysfs_ops = &ab8500_chargalg_sysfs_ops,
- .default_attrs = ab8500_chargalg_chg,
-};
-
-/**
- * ab8500_chargalg_sysfs_exit() - de-init of sysfs entry
- * @di: pointer to the struct ab8500_chargalg
- *
- * This function removes the entry in sysfs.
- */
-static void ab8500_chargalg_sysfs_exit(struct ab8500_chargalg *di)
-{
- kobject_del(&di->chargalg_kobject);
-}
-
-/**
- * ab8500_chargalg_sysfs_init() - init of sysfs entry
- * @di: pointer to the struct ab8500_chargalg
- *
- * This function adds an entry in sysfs.
- * Returns error code in case of failure else 0(on success)
- */
-static int ab8500_chargalg_sysfs_init(struct ab8500_chargalg *di)
-{
- int ret = 0;
-
- ret = kobject_init_and_add(&di->chargalg_kobject,
- &ab8500_chargalg_ktype,
- NULL, "ab8500_chargalg");
- if (ret < 0)
- dev_err(di->dev, "failed to create sysfs entry\n");
-
- return ret;
-}
-/* Exposure to the sysfs interface <<END>> */
-
static int __maybe_unused ab8500_chargalg_resume(struct device *dev)
{
struct ab8500_chargalg *di = dev_get_drvdata(dev);
@@ -1911,7 +1725,6 @@ static int ab8500_chargalg_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct power_supply_config psy_cfg = {};
struct ab8500_chargalg *di;
- int ret = 0;
di = devm_kzalloc(dev, sizeof(*di), GFP_KERNEL);
if (!di)
@@ -1959,26 +1772,14 @@ static int ab8500_chargalg_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, di);
- /* sysfs interface to enable/disable charging from user space */
- ret = ab8500_chargalg_sysfs_init(di);
- if (ret) {
- dev_err(di->dev, "failed to create sysfs entry\n");
- return ret;
- }
-
dev_info(di->dev, "probe success\n");
return component_add(dev, &ab8500_chargalg_component_ops);
}
static int ab8500_chargalg_remove(struct platform_device *pdev)
{
- struct ab8500_chargalg *di = platform_get_drvdata(pdev);
-
component_del(&pdev->dev, &ab8500_chargalg_component_ops);
- /* sysfs interface to enable/disable charging from user space */
- ab8500_chargalg_sysfs_exit(di);
-
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 10/11] power: supply: ab8500_charger: Restrict ADC retrieveal
2022-01-27 1:12 [PATCH 00/11] AB8500 charging fixes Linus Walleij
` (8 preceding siblings ...)
2022-01-27 1:12 ` [PATCH 09/11] power: supply: ab8500_chargalg: Drop enable/disable sysfs Linus Walleij
@ 2022-01-27 1:12 ` Linus Walleij
2022-01-27 1:12 ` [PATCH 11/11] power: supply: ab8500_charger: Fix VBAT interval check Linus Walleij
10 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2022-01-27 1:12 UTC (permalink / raw)
To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Linus Walleij
The AB8505 only has two ADC channels: the voltage and current
provided from VBUS (USB). It does not support AC charging at all.
Make sure we don't try to retrieve the non-existing channels.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/power/supply/ab8500_charger.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/power/supply/ab8500_charger.c b/drivers/power/supply/ab8500_charger.c
index ce074c018dcb..681b53bb0df0 100644
--- a/drivers/power/supply/ab8500_charger.c
+++ b/drivers/power/supply/ab8500_charger.c
@@ -3443,17 +3443,19 @@ static int ab8500_charger_probe(struct platform_device *pdev)
di->parent = dev_get_drvdata(pdev->dev.parent);
/* Get ADC channels */
- di->adc_main_charger_v = devm_iio_channel_get(dev, "main_charger_v");
- if (IS_ERR(di->adc_main_charger_v)) {
- ret = dev_err_probe(dev, PTR_ERR(di->adc_main_charger_v),
- "failed to get ADC main charger voltage\n");
- return ret;
- }
- di->adc_main_charger_c = devm_iio_channel_get(dev, "main_charger_c");
- if (IS_ERR(di->adc_main_charger_c)) {
- ret = dev_err_probe(dev, PTR_ERR(di->adc_main_charger_c),
- "failed to get ADC main charger current\n");
- return ret;
+ if (!is_ab8505(di->parent)) {
+ di->adc_main_charger_v = devm_iio_channel_get(dev, "main_charger_v");
+ if (IS_ERR(di->adc_main_charger_v)) {
+ ret = dev_err_probe(dev, PTR_ERR(di->adc_main_charger_v),
+ "failed to get ADC main charger voltage\n");
+ return ret;
+ }
+ di->adc_main_charger_c = devm_iio_channel_get(dev, "main_charger_c");
+ if (IS_ERR(di->adc_main_charger_c)) {
+ ret = dev_err_probe(dev, PTR_ERR(di->adc_main_charger_c),
+ "failed to get ADC main charger current\n");
+ return ret;
+ }
}
di->adc_vbus_v = devm_iio_channel_get(dev, "vbus_v");
if (IS_ERR(di->adc_vbus_v)) {
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 11/11] power: supply: ab8500_charger: Fix VBAT interval check
2022-01-27 1:12 [PATCH 00/11] AB8500 charging fixes Linus Walleij
` (9 preceding siblings ...)
2022-01-27 1:12 ` [PATCH 10/11] power: supply: ab8500_charger: Restrict ADC retrieveal Linus Walleij
@ 2022-01-27 1:12 ` Linus Walleij
10 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2022-01-27 1:12 UTC (permalink / raw)
To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Linus Walleij
When using USB charging, the AB8500 charger is periodically
checking VBAT for a threshold at 3.8V.
This crashes badly, as the class_for_each_device() was passed
the wrong argument. I think this has maybe worked by chance
in the past because of how the structs were arranged but it
is leading to crashes now.
Fix this up and also switch to using microvolts for the
voltages like the rest of the code.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/power/supply/ab8500_charger.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/power/supply/ab8500_charger.c b/drivers/power/supply/ab8500_charger.c
index 681b53bb0df0..88099cdba8a7 100644
--- a/drivers/power/supply/ab8500_charger.c
+++ b/drivers/power/supply/ab8500_charger.c
@@ -163,7 +163,7 @@ enum ab8500_usb_state {
#define USB_CH_IP_CUR_LVL_1P4 1400000
#define USB_CH_IP_CUR_LVL_1P5 1500000
-#define VBAT_TRESH_IP_CUR_RED 3800
+#define VBAT_TRESH_IP_CUR_RED 3800000
#define to_ab8500_charger_usb_device_info(x) container_of((x), \
struct ab8500_charger, usb_chg)
@@ -1920,7 +1920,11 @@ static int ab8500_charger_get_ext_psy_data(struct device *dev, void *data)
di = to_ab8500_charger_usb_device_info(usb_chg);
- /* For all psy where the driver name appears in any supplied_to */
+ /*
+ * For all psy where the driver name appears in any supplied_to
+ * in practice what we will find will always be "ab8500_fg" as
+ * the fuel gauge is responsible of keeping track of VBAT.
+ */
j = match_string(supplicants, ext->num_supplicants, psy->desc->name);
if (j < 0)
return 0;
@@ -1937,7 +1941,10 @@ static int ab8500_charger_get_ext_psy_data(struct device *dev, void *data)
case POWER_SUPPLY_PROP_VOLTAGE_NOW:
switch (ext->desc->type) {
case POWER_SUPPLY_TYPE_BATTERY:
- di->vbat = ret.intval / 1000;
+ /* This will always be "ab8500_fg" */
+ dev_dbg(di->dev, "get VBAT from %s\n",
+ dev_name(&ext->dev));
+ di->vbat = ret.intval;
break;
default:
break;
@@ -1966,7 +1973,7 @@ static void ab8500_charger_check_vbat_work(struct work_struct *work)
struct ab8500_charger, check_vbat_work.work);
class_for_each_device(power_supply_class, NULL,
- di->usb_chg.psy, ab8500_charger_get_ext_psy_data);
+ &di->usb_chg, ab8500_charger_get_ext_psy_data);
/* First run old_vbat is 0. */
if (di->old_vbat == 0)
@@ -1991,8 +1998,8 @@ static void ab8500_charger_check_vbat_work(struct work_struct *work)
* No need to check the battery voltage every second when not close to
* the threshold.
*/
- if (di->vbat < (VBAT_TRESH_IP_CUR_RED + 100) &&
- (di->vbat > (VBAT_TRESH_IP_CUR_RED - 100)))
+ if (di->vbat < (VBAT_TRESH_IP_CUR_RED + 100000) &&
+ (di->vbat > (VBAT_TRESH_IP_CUR_RED - 100000)))
t = 1;
queue_delayed_work(di->charger_wq, &di->check_vbat_work, t * HZ);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread