All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11 v2] AB8500 charging fixes
@ 2022-01-29  0:49 Linus Walleij
  2022-01-29  0:49 ` [PATCH 01/11 v2] power: supply: ab8500: Drop BATCTRL thermal mode Linus Walleij
                   ` (11 more replies)
  0 siblings, 12 replies; 14+ messages in thread
From: Linus Walleij @ 2022-01-29  0:49 UTC (permalink / raw)
  To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Linus Walleij

This is a first round of AB8500 charging patches for v5.18.

ChangeLog v1->v2:
- Patch 5 contained code based on a patch I will submit in
  the next iteration, now augmented to apply and build
  cleanly.

Linus Walleij (11):
  power: supply: ab8500: Drop BATCTRL thermal mode
  power: supply: ab8500: Swap max and overvoltage
  power: supply: ab8500: Integrate thermal zone
  power: supply: ab8500_fg: Break loop for measurement
  power: supply: ab8500_fg: Break out load compensated voltage
  power: supply: ab8500_fg: Safeguard compensated voltage
  power: supply: ab8500_fg: Drop useless parameter
  power: supply: ab8500_chargalg: Drop charging step
  power: supply: ab8500_chargalg: Drop enable/disable sysfs
  power: supply: ab8500_charger: Restrict ADC retrieveal
  power: supply: ab8500_charger: Fix VBAT interval check

 drivers/power/supply/Kconfig           |   2 +
 drivers/power/supply/ab8500-bm.h       |  49 ----
 drivers/power/supply/ab8500_bmdata.c   |  34 +--
 drivers/power/supply/ab8500_btemp.c    | 330 +++----------------------
 drivers/power/supply/ab8500_chargalg.c | 318 +-----------------------
 drivers/power/supply/ab8500_charger.c  |  43 ++--
 drivers/power/supply/ab8500_fg.c       |  96 ++++---
 7 files changed, 138 insertions(+), 734 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/11 v2] power: supply: ab8500: Drop BATCTRL thermal mode
  2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
@ 2022-01-29  0:49 ` Linus Walleij
  2022-01-29  0:49 ` [PATCH 02/11 v2] power: supply: ab8500: Swap max and overvoltage Linus Walleij
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2022-01-29  0:49 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>
---
ChangeLog v1->v2:
- Resending with other patches, no changes.
---
 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] 14+ messages in thread

* [PATCH 02/11 v2] power: supply: ab8500: Swap max and overvoltage
  2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
  2022-01-29  0:49 ` [PATCH 01/11 v2] power: supply: ab8500: Drop BATCTRL thermal mode Linus Walleij
@ 2022-01-29  0:49 ` Linus Walleij
  2022-01-29  0:49 ` [PATCH 03/11 v2] power: supply: ab8500: Integrate thermal zone Linus Walleij
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2022-01-29  0:49 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>
---
ChangeLog v1->v2:
- Resending with other patches, no changes.
---
 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] 14+ messages in thread

* [PATCH 03/11 v2] power: supply: ab8500: Integrate thermal zone
  2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
  2022-01-29  0:49 ` [PATCH 01/11 v2] power: supply: ab8500: Drop BATCTRL thermal mode Linus Walleij
  2022-01-29  0:49 ` [PATCH 02/11 v2] power: supply: ab8500: Swap max and overvoltage Linus Walleij
@ 2022-01-29  0:49 ` Linus Walleij
  2022-01-29  0:49 ` [PATCH 04/11 v2] power: supply: ab8500_fg: Break loop for measurement Linus Walleij
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2022-01-29  0:49 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>
---
ChangeLog v1->v2:
- Resending with other patches, no changes.
---
 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] 14+ messages in thread

* [PATCH 04/11 v2] power: supply: ab8500_fg: Break loop for measurement
  2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
                   ` (2 preceding siblings ...)
  2022-01-29  0:49 ` [PATCH 03/11 v2] power: supply: ab8500: Integrate thermal zone Linus Walleij
@ 2022-01-29  0:49 ` Linus Walleij
  2022-01-29  0:49 ` [PATCH 05/11 v2] power: supply: ab8500_fg: Break out load compensated voltage Linus Walleij
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2022-01-29  0:49 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>
---
ChangeLog v1->v2:
- Resending with other patches, no changes.
---
 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] 14+ messages in thread

* [PATCH 05/11 v2] power: supply: ab8500_fg: Break out load compensated voltage
  2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
                   ` (3 preceding siblings ...)
  2022-01-29  0:49 ` [PATCH 04/11 v2] power: supply: ab8500_fg: Break loop for measurement Linus Walleij
@ 2022-01-29  0:49 ` Linus Walleij
  2022-02-11 19:32   ` Sebastian Reichel
  2022-01-29  0:49 ` [PATCH 06/11 v2] power: supply: ab8500_fg: Safeguard " Linus Walleij
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 14+ messages in thread
From: Linus Walleij @ 2022-01-29  0:49 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>
---
ChangeLog v1->v2:
- Fix patch ordering issue: I was using a uv argument to
  ab8500_fg_battery_resistance() however that refactoring is
  later on in the series. Fixed now.
---
 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..1797518c4b0e 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 - (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] 14+ messages in thread

* [PATCH 06/11 v2] power: supply: ab8500_fg: Safeguard compensated voltage
  2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
                   ` (4 preceding siblings ...)
  2022-01-29  0:49 ` [PATCH 05/11 v2] power: supply: ab8500_fg: Break out load compensated voltage Linus Walleij
@ 2022-01-29  0:49 ` Linus Walleij
  2022-01-29  0:49 ` [PATCH 07/11 v2] power: supply: ab8500_fg: Drop useless parameter Linus Walleij
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2022-01-29  0:49 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>
---
ChangeLog v1->v2:
- Resending with other patches, no changes.
---
 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 1797518c4b0e..c659fdc8babd 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] 14+ messages in thread

* [PATCH 07/11 v2] power: supply: ab8500_fg: Drop useless parameter
  2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
                   ` (5 preceding siblings ...)
  2022-01-29  0:49 ` [PATCH 06/11 v2] power: supply: ab8500_fg: Safeguard " Linus Walleij
@ 2022-01-29  0:49 ` Linus Walleij
  2022-01-29  0:49 ` [PATCH 08/11 v2] power: supply: ab8500_chargalg: Drop charging step Linus Walleij
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2022-01-29  0:49 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>
---
ChangeLog v1->v2:
- Resending with other patches, no changes.
---
 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 c659fdc8babd..6436861db016 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] 14+ messages in thread

* [PATCH 08/11 v2] power: supply: ab8500_chargalg: Drop charging step
  2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
                   ` (6 preceding siblings ...)
  2022-01-29  0:49 ` [PATCH 07/11 v2] power: supply: ab8500_fg: Drop useless parameter Linus Walleij
@ 2022-01-29  0:49 ` Linus Walleij
  2022-01-29  0:49 ` [PATCH 09/11 v2] power: supply: ab8500_chargalg: Drop enable/disable sysfs Linus Walleij
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2022-01-29  0:49 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>
---
ChangeLog v1->v2:
- Resending with other patches, no changes.
---
 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, &param);
-	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] 14+ messages in thread

* [PATCH 09/11 v2] power: supply: ab8500_chargalg: Drop enable/disable sysfs
  2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
                   ` (7 preceding siblings ...)
  2022-01-29  0:49 ` [PATCH 08/11 v2] power: supply: ab8500_chargalg: Drop charging step Linus Walleij
@ 2022-01-29  0:49 ` Linus Walleij
  2022-01-29  0:49 ` [PATCH 10/11 v2] power: supply: ab8500_charger: Restrict ADC retrieveal Linus Walleij
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2022-01-29  0:49 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>
---
ChangeLog v1->v2:
- Resending with other patches, no changes.
---
 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, &param);
-	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] 14+ messages in thread

* [PATCH 10/11 v2] power: supply: ab8500_charger: Restrict ADC retrieveal
  2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
                   ` (8 preceding siblings ...)
  2022-01-29  0:49 ` [PATCH 09/11 v2] power: supply: ab8500_chargalg: Drop enable/disable sysfs Linus Walleij
@ 2022-01-29  0:49 ` Linus Walleij
  2022-01-29  0:49 ` [PATCH 11/11 v2] power: supply: ab8500_charger: Fix VBAT interval check Linus Walleij
  2022-02-11 19:29 ` [PATCH 00/11 v2] AB8500 charging fixes Sebastian Reichel
  11 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2022-01-29  0:49 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>
---
ChangeLog v1->v2:
- Resending with other patches, no changes.
---
 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] 14+ messages in thread

* [PATCH 11/11 v2] power: supply: ab8500_charger: Fix VBAT interval check
  2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
                   ` (9 preceding siblings ...)
  2022-01-29  0:49 ` [PATCH 10/11 v2] power: supply: ab8500_charger: Restrict ADC retrieveal Linus Walleij
@ 2022-01-29  0:49 ` Linus Walleij
  2022-02-11 19:29 ` [PATCH 00/11 v2] AB8500 charging fixes Sebastian Reichel
  11 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2022-01-29  0:49 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>
---
ChangeLog v1->v2:
- Resending with other patches, no changes.
---
 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] 14+ messages in thread

* Re: [PATCH 00/11 v2] AB8500 charging fixes
  2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
                   ` (10 preceding siblings ...)
  2022-01-29  0:49 ` [PATCH 11/11 v2] power: supply: ab8500_charger: Fix VBAT interval check Linus Walleij
@ 2022-02-11 19:29 ` Sebastian Reichel
  11 siblings, 0 replies; 14+ messages in thread
From: Sebastian Reichel @ 2022-02-11 19:29 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Marcus Cooper, linux-pm

[-- Attachment #1: Type: text/plain, Size: 1470 bytes --]

Hi,

On Sat, Jan 29, 2022 at 01:49:14AM +0100, Linus Walleij wrote:
> This is a first round of AB8500 charging patches for v5.18.
> 
> ChangeLog v1->v2:
> - Patch 5 contained code based on a patch I will submit in
>   the next iteration, now augmented to apply and build
>   cleanly.
> 
> Linus Walleij (11):
>   power: supply: ab8500: Drop BATCTRL thermal mode
>   power: supply: ab8500: Swap max and overvoltage
>   power: supply: ab8500: Integrate thermal zone
>   power: supply: ab8500_fg: Break loop for measurement
>   power: supply: ab8500_fg: Break out load compensated voltage
>   power: supply: ab8500_fg: Safeguard compensated voltage
>   power: supply: ab8500_fg: Drop useless parameter
>   power: supply: ab8500_chargalg: Drop charging step
>   power: supply: ab8500_chargalg: Drop enable/disable sysfs
>   power: supply: ab8500_charger: Restrict ADC retrieveal
>   power: supply: ab8500_charger: Fix VBAT interval check
> 
>  drivers/power/supply/Kconfig           |   2 +
>  drivers/power/supply/ab8500-bm.h       |  49 ----
>  drivers/power/supply/ab8500_bmdata.c   |  34 +--
>  drivers/power/supply/ab8500_btemp.c    | 330 +++----------------------
>  drivers/power/supply/ab8500_chargalg.c | 318 +-----------------------
>  drivers/power/supply/ab8500_charger.c  |  43 ++--
>  drivers/power/supply/ab8500_fg.c       |  96 ++++---
>  7 files changed, 138 insertions(+), 734 deletions(-)

Thanks, queued.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 05/11 v2] power: supply: ab8500_fg: Break out load compensated voltage
  2022-01-29  0:49 ` [PATCH 05/11 v2] power: supply: ab8500_fg: Break out load compensated voltage Linus Walleij
@ 2022-02-11 19:32   ` Sebastian Reichel
  0 siblings, 0 replies; 14+ messages in thread
From: Sebastian Reichel @ 2022-02-11 19:32 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Marcus Cooper, linux-pm

[-- Attachment #1: Type: text/plain, Size: 1764 bytes --]

Hi,

On Sat, Jan 29, 2022 at 01:49:19AM +0100, Linus Walleij wrote:
> 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>
> ---
> ChangeLog v1->v2:
> - Fix patch ordering issue: I was using a uv argument to
>   ab8500_fg_battery_resistance() however that refactoring is
>   later on in the series. Fixed now.
> ---
>  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..1797518c4b0e 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.

This documents the always parameter, which is only introduced in the
next patch :) Since I found nothing else I merged the patchset anyways.

I also noticed that 'always' is always true at the end of this
patchset (and assumed it will be used in a future patchset).

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-02-11 19:32 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-29  0:49 [PATCH 00/11 v2] AB8500 charging fixes Linus Walleij
2022-01-29  0:49 ` [PATCH 01/11 v2] power: supply: ab8500: Drop BATCTRL thermal mode Linus Walleij
2022-01-29  0:49 ` [PATCH 02/11 v2] power: supply: ab8500: Swap max and overvoltage Linus Walleij
2022-01-29  0:49 ` [PATCH 03/11 v2] power: supply: ab8500: Integrate thermal zone Linus Walleij
2022-01-29  0:49 ` [PATCH 04/11 v2] power: supply: ab8500_fg: Break loop for measurement Linus Walleij
2022-01-29  0:49 ` [PATCH 05/11 v2] power: supply: ab8500_fg: Break out load compensated voltage Linus Walleij
2022-02-11 19:32   ` Sebastian Reichel
2022-01-29  0:49 ` [PATCH 06/11 v2] power: supply: ab8500_fg: Safeguard " Linus Walleij
2022-01-29  0:49 ` [PATCH 07/11 v2] power: supply: ab8500_fg: Drop useless parameter Linus Walleij
2022-01-29  0:49 ` [PATCH 08/11 v2] power: supply: ab8500_chargalg: Drop charging step Linus Walleij
2022-01-29  0:49 ` [PATCH 09/11 v2] power: supply: ab8500_chargalg: Drop enable/disable sysfs Linus Walleij
2022-01-29  0:49 ` [PATCH 10/11 v2] power: supply: ab8500_charger: Restrict ADC retrieveal Linus Walleij
2022-01-29  0:49 ` [PATCH 11/11 v2] power: supply: ab8500_charger: Fix VBAT interval check Linus Walleij
2022-02-11 19:29 ` [PATCH 00/11 v2] AB8500 charging fixes Sebastian Reichel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.