[PATCH 0/5] Fix Incorrect State of Charge for RK817

[PATCH 0/5] Fix Incorrect State of Charge for RK817

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Users of the popular firmware RockNIX had identified a few bugs with
accurately displaying the correct state of charge for RK817 battery
devices. With the help of one user analyzing the code with Claude
they were able to identify a few potential bugs along with potential
solutions which I was then able to verify. These bugs are as follows:

1) Incorrectly clamping a value with a max of 10000 instead of 100000.
This bug caused the state of charge to sometimes read as 10% instead
of 100% as it should have.

2) Using the boot_voltage value to calibrate the battery in scenarios
where it could not be trusted. The boot_voltage register populates
immediately at power-on, and the driver would check if this value
was older than 30 minutes before using it. Unfortunately when combined
with the default behavior of the bootloader to turn the device off
when plugged in (requiring a user to explicitly power the device on)
the boot_voltage and boot_timer values were wrong. The boot voltage
would not account for any voltage added while the device was off,
and the boot_counter that tracks the time the device was off would
be reset. To fix this we adopted the same behavior as the BSP driver
which would typically (outside of first-boot scenarios) use a value
saved to NVRAM or the value from the coulomb counter directly and
then recalibrate with the OCV voltage once it was available.

3) A similar issue would happen during suspend, as values from the
coulomb counter could not be relied upon. So we add suspend specific
recalibration to the driver.

Included in this series is another fix that would cause the battery
to drain while the system was shut down. This patch replicates the
Rockchip BSP driver to set a shutdown register and provides a
device-tree parameter to override the behavior (though I am not aware
of any existing boards that need the override).

Chris Morgan (5):
  power: supply: rk817-charger: Fix NVRAM SoC Value
  power: supply: rk817-charger: Resolve boot state of charge from the
    coulomb counter
  power: supply: rk817-charger: Correct the gauge across system sleep
  dt-bindings: power: supply: rk817-charger: Add
    rockchip,gate-function-disable
  power: supply: rk817-charger: Clear SYS_CAN_SD register by default

 .../bindings/mfd/rockchip,rk817.yaml          |   7 +
 drivers/power/supply/rk817_charger.c          | 336 +++++++++++++++---
 include/linux/mfd/rk808.h                     |   3 +
 3 files changed, 290 insertions(+), 56 deletions(-)

-- 
2.53.0

[PATCH 0/5] Fix Incorrect State of Charge for RK817

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Users of the popular firmware RockNIX had identified a few bugs with
accurately displaying the correct state of charge for RK817 battery
devices. With the help of one user analyzing the code with Claude
they were able to identify a few potential bugs along with potential
solutions which I was then able to verify. These bugs are as follows:

1) Incorrectly clamping a value with a max of 10000 instead of 100000.
This bug caused the state of charge to sometimes read as 10% instead
of 100% as it should have.

2) Using the boot_voltage value to calibrate the battery in scenarios
where it could not be trusted. The boot_voltage register populates
immediately at power-on, and the driver would check if this value
was older than 30 minutes before using it. Unfortunately when combined
with the default behavior of the bootloader to turn the device off
when plugged in (requiring a user to explicitly power the device on)
the boot_voltage and boot_timer values were wrong. The boot voltage
would not account for any voltage added while the device was off,
and the boot_counter that tracks the time the device was off would
be reset. To fix this we adopted the same behavior as the BSP driver
which would typically (outside of first-boot scenarios) use a value
saved to NVRAM or the value from the coulomb counter directly and
then recalibrate with the OCV voltage once it was available.

3) A similar issue would happen during suspend, as values from the
coulomb counter could not be relied upon. So we add suspend specific
recalibration to the driver.

Included in this series is another fix that would cause the battery
to drain while the system was shut down. This patch replicates the
Rockchip BSP driver to set a shutdown register and provides a
device-tree parameter to override the behavior (though I am not aware
of any existing boards that need the override).

Chris Morgan (5):
  power: supply: rk817-charger: Fix NVRAM SoC Value
  power: supply: rk817-charger: Resolve boot state of charge from the
    coulomb counter
  power: supply: rk817-charger: Correct the gauge across system sleep
  dt-bindings: power: supply: rk817-charger: Add
    rockchip,gate-function-disable
  power: supply: rk817-charger: Clear SYS_CAN_SD register by default

 .../bindings/mfd/rockchip,rk817.yaml          |   7 +
 drivers/power/supply/rk817_charger.c          | 336 +++++++++++++++---
 include/linux/mfd/rk808.h                     |   3 +
 3 files changed, 290 insertions(+), 56 deletions(-)

-- 
2.53.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 1/5] power: supply: rk817-charger: Fix NVRAM SoC Value

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

The NVRAM value is stored as a value between 0 (0%) and 100000 (100%),
but the bounds checking checks and truncates with a max of 10000,
causing a value of 100% to get read as 10%.

Fixes: 11cb8da0189b ("power: supply: Add charger driver for Rockchip RK817")
Suggested-by: Jacob Cook <jacobmcook1984@gmail.com>
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/power/supply/rk817_charger.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
index 5558b182a1a6..86da0462fb59 100644
--- a/drivers/power/supply/rk817_charger.c
+++ b/drivers/power/supply/rk817_charger.c
@@ -728,7 +728,7 @@ static int rk817_read_battery_nvram_values(struct rk817_charger *charger)
 
 	/*
 	 * Read the nvram for state of charge. Sanity check for values greater
-	 * than 100 (10000) or less than 0, because other things (BSP kernels,
+	 * than 100 (100000) or less than 0, because other things (BSP kernels,
 	 * U-Boot, or even i2cset) can write to this register. If the value is
 	 * off it should get corrected automatically when the voltage drops to
 	 * the min (soc is 0) or when the battery is full (soc is 100).
@@ -738,8 +738,8 @@ static int rk817_read_battery_nvram_values(struct rk817_charger *charger)
 	if (ret < 0)
 		return ret;
 	charger->soc = get_unaligned_le24(bulk_reg);
-	if (charger->soc > 10000)
-		charger->soc = 10000;
+	if (charger->soc > 100000)
+		charger->soc = 100000;
 	if (charger->soc < 0)
 		charger->soc = 0;
 
-- 
2.53.0

[PATCH 1/5] power: supply: rk817-charger: Fix NVRAM SoC Value

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

The NVRAM value is stored as a value between 0 (0%) and 100000 (100%),
but the bounds checking checks and truncates with a max of 10000,
causing a value of 100% to get read as 10%.

Fixes: 11cb8da0189b ("power: supply: Add charger driver for Rockchip RK817")
Suggested-by: Jacob Cook <jacobmcook1984@gmail.com>
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/power/supply/rk817_charger.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
index 5558b182a1a6..86da0462fb59 100644
--- a/drivers/power/supply/rk817_charger.c
+++ b/drivers/power/supply/rk817_charger.c
@@ -728,7 +728,7 @@ static int rk817_read_battery_nvram_values(struct rk817_charger *charger)
 
 	/*
 	 * Read the nvram for state of charge. Sanity check for values greater
-	 * than 100 (10000) or less than 0, because other things (BSP kernels,
+	 * than 100 (100000) or less than 0, because other things (BSP kernels,
 	 * U-Boot, or even i2cset) can write to this register. If the value is
 	 * off it should get corrected automatically when the voltage drops to
 	 * the min (soc is 0) or when the battery is full (soc is 100).
@@ -738,8 +738,8 @@ static int rk817_read_battery_nvram_values(struct rk817_charger *charger)
 	if (ret < 0)
 		return ret;
 	charger->soc = get_unaligned_le24(bulk_reg);
-	if (charger->soc > 10000)
-		charger->soc = 10000;
+	if (charger->soc > 100000)
+		charger->soc = 100000;
 	if (charger->soc < 0)
 		charger->soc = 0;
 
-- 
2.53.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 2/5] power: supply: rk817-charger: Resolve boot state of charge from the coulomb counter

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

The boot state of charge comes from PWRON_VOL when OFF_CNT reads three
or more and from the coulomb counter otherwise, and both fail in the
field.

PWRON_VOL is sampled at the power-on instant. A power-on caused by
plugging in a charger samples under charge current: on an RG353M it
read above the pack's maximum design voltage while the pack was at
74%. An RG DS was seen holding 2.63V in the register while running.
Either becomes a wrong percentage on every boot that reaches it.

A negative counter was clamped to zero and reported as 0%.

The counter itself keeps counting while the system is off: the same
RG353M, saved at 74% and charged for 1.5 hours powered off, booted
with the counter at full. The BSP driver relies on this: it reads
OFF_CNT into a flag it never uses, consults PWRON_VOL only when
BAT_CON marks a new battery, and adopts the counter when it diverges
from the saved capacity ("system halt last time").

Do the same. Keep the PWRON_VOL lookup for the first boot. Otherwise
take the counter, capped at full, unless it reads zero or negative,
in which case keep the saved soc, which is at most one update
interval old. Then read the
relax voltage pair every update interval and, while the battery is
resting and the pair disagrees with the tracked soc by more than 5%,
re-init the counter from it. The pair is a true open circuit voltage,
so this corrects a counter that is wrong by any amount, and it is the
mechanism the BSP driver uses at resume.

Expose PWRON_VOL as POWER_SUPPLY_PROP_VOLTAGE_BOOT and the current
rested pair as POWER_SUPPLY_PROP_VOLTAGE_OCV, -ENODATA while the
battery is not resting.

Log the counter, saved soc and battery voltage at boot, as the BSP logs
its saved and present capacity, so a counter that diverged from the
saved soc is visible.

Fixes: 11cb8da0189b ("power: supply: Add charger driver for Rockchip RK817")
Suggested-by: Jacob Cook <jacobmcook1984@gmail.com>
Co-developed-by: Jacob Cook <jacobmcook1984@gmail.com>
Signed-off-by: Jacob Cook <jacobmcook1984@gmail.com>
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Assisted-by: Claude:claude-5-fable
---
 drivers/power/supply/rk817_charger.c | 222 ++++++++++++++++++++-------
 include/linux/mfd/rk808.h            |   2 +
 2 files changed, 171 insertions(+), 53 deletions(-)

diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
index 86da0462fb59..50fbea298127 100644
--- a/drivers/power/supply/rk817_charger.c
+++ b/drivers/power/supply/rk817_charger.c
@@ -95,6 +95,15 @@ struct rk817_charger {
 	int bat_voltage_min_design_uv;
 	int bat_voltage_max_design_uv;
 
+	/* Battery voltage the PMIC sampled when the system powered on. */
+	int boot_voltage_uv;
+
+	/* Rested voltage the gauge latched, 0 if it took no usable pair. */
+	int relax_voltage_uv;
+
+	/* Retained for the ocv-capacity table the recalibration needs. */
+	struct power_supply_battery_info *bat_info;
+
 	/* Values updated periodically by driver for display. */
 	int charge_now_uah;
 	int volt_avg_uv;
@@ -332,6 +341,42 @@ static int rk817_bat_calib_cap(struct rk817_charger *charger)
 	return 0;
 }
 
+/* Calibrated microvolts from one of the gas gauge's 16 bit voltage registers. */
+static int rk817_bat_voltage_uv(struct rk817_charger *charger, unsigned int reg)
+{
+	u8 bulk_reg[2];
+
+	regmap_bulk_read(charger->rk808->regmap, reg, bulk_reg, 2);
+
+	return (charger->voltage_k * get_unaligned_be16(bulk_reg)) +
+		1000 * charger->voltage_b;
+}
+
+/*
+ * Voltage the gauge latched while the battery was resting. Relax mode only
+ * samples below RELAX_THRE_CUR, so this is a true open circuit voltage with
+ * no resistance term. Returns 0 unless the battery is resting now and the
+ * latched pair is inside the pack's design range.
+ */
+static int rk817_bat_relax_voltage_uv(struct rk817_charger *charger)
+{
+	unsigned int need = RK817_RELAX_STS | RK817_RELAX_VOL_UPD;
+	int ret, sts, vol;
+
+	ret = regmap_read(charger->rk808->regmap, RK817_GAS_GAUGE_GG_STS, &sts);
+	if (ret < 0 || (sts & need) != need)
+		return 0;
+
+	vol = max(rk817_bat_voltage_uv(charger, RK817_GAS_GAUGE_RELAX_VOL1_H),
+		  rk817_bat_voltage_uv(charger, RK817_GAS_GAUGE_RELAX_VOL2_H));
+
+	if (vol < charger->bat_voltage_min_design_uv ||
+	    vol > charger->bat_voltage_max_design_uv)
+		return 0;
+
+	return vol;
+}
+
 static void rk817_read_props(struct rk817_charger *charger)
 {
 	int tmp, reg;
@@ -367,11 +412,8 @@ static void rk817_read_props(struct rk817_charger *charger)
 	charger->soc = charger->charge_now_uah * 100 / charger->fcc_mah;
 
 	/* Update reported voltage. */
-	regmap_bulk_read(charger->rk808->regmap, RK817_GAS_GAUGE_BAT_VOL_H,
-			 bulk_reg, 2);
-	tmp = get_unaligned_be16(bulk_reg);
-	charger->volt_avg_uv = (charger->voltage_k * tmp) + 1000 *
-				charger->voltage_b;
+	charger->volt_avg_uv = rk817_bat_voltage_uv(charger,
+						    RK817_GAS_GAUGE_BAT_VOL_H);
 
 	/*
 	 * Update reported current. Note value from registers is a signed 16
@@ -430,6 +472,8 @@ static void rk817_read_props(struct rk817_charger *charger)
 		charger->charger_input_volt_avg_uv = 0;
 	}
 
+	charger->relax_voltage_uv = rk817_bat_relax_voltage_uv(charger);
+
 	/* Calibrate battery capacity and soc. */
 	rk817_bat_calib_cap(charger);
 }
@@ -516,6 +560,14 @@ static int rk817_bat_get_prop(struct power_supply *ps,
 	case POWER_SUPPLY_PROP_VOLTAGE_AVG:
 		val->intval = charger->volt_avg_uv;
 		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_BOOT:
+		val->intval = charger->boot_voltage_uv;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_OCV:
+		if (!charger->relax_voltage_uv)
+			return -ENODATA;
+		val->intval = charger->relax_voltage_uv;
+		break;
 	case POWER_SUPPLY_PROP_CURRENT_AVG:
 		val->intval = charger->cur_avg_ua;
 		break;
@@ -654,6 +706,8 @@ static enum power_supply_property rk817_bat_props[] = {
 	POWER_SUPPLY_PROP_CHARGE_NOW,
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
 	POWER_SUPPLY_PROP_VOLTAGE_AVG,
+	POWER_SUPPLY_PROP_VOLTAGE_BOOT,
+	POWER_SUPPLY_PROP_VOLTAGE_OCV,
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
 	POWER_SUPPLY_PROP_CURRENT_AVG,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
@@ -746,15 +800,50 @@ static int rk817_read_battery_nvram_values(struct rk817_charger *charger)
 	return 0;
 }
 
+/*
+ * The relax pair is a true rested voltage: where it disagrees with the
+ * tracked soc by more than 5% (chosen arbitrarily), re-init the counter
+ * from it.
+ */
+static void rk817_bat_ocv_recalibrate(struct rk817_charger *charger)
+{
+	struct rk808 *rk808 = charger->rk808;
+	int ocv_soc, charge_now_adc;
+	u8 bulk_reg[4];
+
+	if (!charger->battery_present)
+		return;
+
+	if (!charger->relax_voltage_uv)
+		return;
+
+	ocv_soc = power_supply_batinfo_ocv2cap(charger->bat_info,
+					       charger->relax_voltage_uv,
+					       20) * 1000;
+	if (ocv_soc < 0 || abs(ocv_soc - charger->soc) <= 5000)
+		return;
+
+	dev_dbg(charger->dev,
+		"OCV %duV disagrees with soc, recalibrating (%d -> %d)\n",
+		charger->relax_voltage_uv, charger->soc, ocv_soc);
+	charger->soc = ocv_soc;
+	charge_now_adc = CHARGE_TO_ADC(ocv_soc * charger->fcc_mah / 100000,
+				       charger->res_div);
+	put_unaligned_be32(charge_now_adc, bulk_reg);
+	regmap_bulk_write(rk808->regmap, RK817_GAS_GAUGE_Q_INIT_H3,
+			  bulk_reg, 4);
+	rk817_record_battery_nvram_values(charger);
+}
+
 static int
 rk817_read_or_set_full_charge_on_boot(struct rk817_charger *charger,
 				struct power_supply_battery_info *bat_info)
 {
 	struct rk808 *rk808 = charger->rk808;
+	u32 boot_charge_mah;
+	int ret, reg, tmp, off_cnt, bat_uv, boot_soc, table_soc;
+	bool first_boot, keep_saved;
 	u8 bulk_reg[4];
-	u32 boot_voltage, boot_charge_mah;
-	int ret, reg, off_time, tmp;
-	bool first_boot;
 
 	/*
 	 * Check if the battery is uninitalized. If it is, the columb counter
@@ -764,25 +853,22 @@ rk817_read_or_set_full_charge_on_boot(struct rk817_charger *charger,
 	if (ret < 0)
 		return ret;
 	first_boot = reg & RK817_BAT_CON;
-	/*
-	 * If the battery is uninitialized, use the poweron voltage and an ocv
-	 * lookup to guess our charge. The number won't be very accurate until
-	 * we hit either our minimum voltage (0%) or full charge (100%).
-	 */
+
+	/* PWRON_VOL only refreshes on a real power-on; bound it to the pack. */
+	charger->boot_voltage_uv =
+		clamp(rk817_bat_voltage_uv(charger, RK817_GAS_GAUGE_PWRON_VOL_H),
+		      bat_info->voltage_min_design_uv,
+		      bat_info->voltage_max_design_uv);
+
 	if (first_boot) {
-		regmap_bulk_read(rk808->regmap, RK817_GAS_GAUGE_PWRON_VOL_H,
-				 bulk_reg, 2);
-		tmp = get_unaligned_be16(bulk_reg);
-		boot_voltage = (charger->voltage_k * tmp) +
-				1000 * charger->voltage_b;
 		/*
-		 * Since only implementation has no working thermistor, assume
-		 * 20C for OCV lookup. If lookup fails, report error with OCV
-		 * table.
+		 * Nothing has been saved yet, so estimate from the boot
+		 * voltage with an ocv lookup. Since only implementation has
+		 * no working thermistor, assume 20C for that lookup.
 		 */
 		charger->soc = power_supply_batinfo_ocv2cap(bat_info,
-							    boot_voltage,
-							    20) * 1000;
+						charger->boot_voltage_uv,
+						20) * 1000;
 		if (charger->soc < 0)
 			charger->soc = 0;
 
@@ -803,39 +889,54 @@ rk817_read_or_set_full_charge_on_boot(struct rk817_charger *charger,
 		if (ret < 0)
 			return ret;
 
+		/*
+		 * The coulomb counter keeps counting while the system is off,
+		 * so take it, capped at full, unless it reads invalid; the
+		 * saved soc is then at most one update interval old. A counter
+		 * that is merely wrong is corrected by the next rested relax
+		 * pair.
+		 */
 		regmap_bulk_read(rk808->regmap, RK817_GAS_GAUGE_Q_PRES_H3,
 				 bulk_reg, 4);
 		tmp = get_unaligned_be32(bulk_reg);
-		if (tmp < 0)
-			tmp = 0;
-		boot_charge_mah = ADC_TO_CHARGE_UAH(tmp,
-						    charger->res_div) / 1000;
-		/*
-		 * Check if the columb counter has been off for more than 30
-		 * minutes as it tends to drift downward. If so, re-init soc
-		 * with the boot voltage instead. Note the unit values for the
-		 * OFF_CNT register appear to be in decaminutes and stops
-		 * counting at 2550 (0xFF) minutes. BSP kernel used OCV, but
-		 * for me occasionally that would show invalid values. Boot
-		 * voltage is only accurate for me on first poweron (not
-		 * reboots), but we shouldn't ever encounter an OFF_CNT more
-		 * than 0 on a reboot anyway.
-		 */
-		regmap_read(rk808->regmap, RK817_GAS_GAUGE_OFF_CNT, &off_time);
-		if (off_time >= 3) {
-			regmap_bulk_read(rk808->regmap,
-					 RK817_GAS_GAUGE_PWRON_VOL_H,
-					 bulk_reg, 2);
-			tmp = get_unaligned_be16(bulk_reg);
-			boot_voltage = (charger->voltage_k * tmp) +
-					1000 * charger->voltage_b;
-			charger->soc =
-				power_supply_batinfo_ocv2cap(bat_info,
-							     boot_voltage,
-							     20) * 1000;
+		regmap_read(rk808->regmap, RK817_GAS_GAUGE_OFF_CNT, &off_cnt);
+		bat_uv = rk817_bat_voltage_uv(charger, RK817_GAS_GAUGE_BAT_VOL_H);
+		dev_dbg(charger->dev,
+			"boot: counter %d uAh, saved soc %d, fcc %d mAh, battery %d uV, off_cnt %d\n",
+			tmp > 0 ? ADC_TO_CHARGE_UAH(tmp, charger->res_div) : 0,
+			charger->soc, charger->fcc_mah, bat_uv, off_cnt);
+		if (tmp > 0) {
+			boot_charge_mah = ADC_TO_CHARGE_UAH(tmp,
+							    charger->res_div) / 1000;
+			if (boot_charge_mah > charger->fcc_mah)
+				boot_charge_mah = charger->fcc_mah;
+			boot_soc = boot_charge_mah * 100000 / charger->fcc_mah;
+
+			/*
+			 * The gauge re-derives this count from its own ocv
+			 * model while the pack rests, so bound it with the ocv
+			 * table: under load the terminal voltage sits below the
+			 * open circuit voltage, making the table a floor, and
+			 * under charge above it, making it a ceiling on any
+			 * saved soc that would displace the counter.
+			 */
+			table_soc = power_supply_batinfo_ocv2cap(bat_info, bat_uv,
+								 20) * 1000;
+			if (charger->plugged_in)
+				keep_saved = boot_soc < charger->soc - 10000 &&
+					     charger->soc <= table_soc + 10000;
+			else
+				keep_saved = boot_soc < table_soc - 10000;
+
+			if (keep_saved)
+				dev_dbg(charger->dev,
+					"counter %d, voltage allows %d, keeping saved soc %d\n",
+					boot_soc, table_soc, charger->soc);
+			else
+				charger->soc = boot_soc;
 		} else {
-			charger->soc = (boot_charge_mah * 1000 * 100 /
-					charger->fcc_mah);
+			dev_dbg(charger->dev,
+				"coulomb counter invalid, keeping saved soc\n");
 		}
 	}
 
@@ -1044,6 +1145,7 @@ static void rk817_charging_monitor(struct work_struct *work)
 	charger = container_of(work, struct rk817_charger, work.work);
 
 	rk817_read_props(charger);
+	rk817_bat_ocv_recalibrate(charger);
 
 	/* Run every 8 seconds like the BSP driver did. */
 	queue_delayed_work(system_percpu_wq, &charger->work, msecs_to_jiffies(8000));
@@ -1056,6 +1158,13 @@ static void rk817_cleanup_node(void *data)
 	of_node_put(node);
 }
 
+static void rk817_put_battery_info(void *data)
+{
+	struct rk817_charger *charger = data;
+
+	power_supply_put_battery_info(charger->bat_ps, charger->bat_info);
+}
+
 static int rk817_charger_probe(struct platform_device *pdev)
 {
 	struct rk808 *rk808 = dev_get_drvdata(pdev->dev.parent);
@@ -1172,7 +1281,14 @@ static int rk817_charger_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	power_supply_put_battery_info(charger->bat_ps, bat_info);
+	/*
+	 * Keep the battery info: the periodic recalibration needs its
+	 * ocv-capacity table for as long as the driver is bound.
+	 */
+	charger->bat_info = bat_info;
+	ret = devm_add_action_or_reset(dev, rk817_put_battery_info, charger);
+	if (ret)
+		return ret;
 
 	plugin_irq = platform_get_irq(pdev, 0);
 	if (plugin_irq < 0)
diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
index 7ffc904c864c..72419d311626 100644
--- a/include/linux/mfd/rk808.h
+++ b/include/linux/mfd/rk808.h
@@ -1211,6 +1211,8 @@ enum rk809_reg_id {
 
 #define RK817_GAS_GAUGE_RELAX_THRE_H	0x58
 #define RK817_GAS_GAUGE_RELAX_THRE_L	0x59
+#define RK817_GAS_GAUGE_RELAX_VOL1_H	0x5a
+#define RK817_GAS_GAUGE_RELAX_VOL2_H	0x5c
 #define RK817_GAS_GAUGE_OCV_THRE_VOL	0x62
 #define RK817_GAS_GAUGE_OCV_VOL_H	0x63
 #define RK817_GAS_GAUGE_OCV_VOL_L	0x64
-- 
2.53.0

[PATCH 2/5] power: supply: rk817-charger: Resolve boot state of charge from the coulomb counter

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

The boot state of charge comes from PWRON_VOL when OFF_CNT reads three
or more and from the coulomb counter otherwise, and both fail in the
field.

PWRON_VOL is sampled at the power-on instant. A power-on caused by
plugging in a charger samples under charge current: on an RG353M it
read above the pack's maximum design voltage while the pack was at
74%. An RG DS was seen holding 2.63V in the register while running.
Either becomes a wrong percentage on every boot that reaches it.

A negative counter was clamped to zero and reported as 0%.

The counter itself keeps counting while the system is off: the same
RG353M, saved at 74% and charged for 1.5 hours powered off, booted
with the counter at full. The BSP driver relies on this: it reads
OFF_CNT into a flag it never uses, consults PWRON_VOL only when
BAT_CON marks a new battery, and adopts the counter when it diverges
from the saved capacity ("system halt last time").

Do the same. Keep the PWRON_VOL lookup for the first boot. Otherwise
take the counter, capped at full, unless it reads zero or negative,
in which case keep the saved soc, which is at most one update
interval old. Then read the
relax voltage pair every update interval and, while the battery is
resting and the pair disagrees with the tracked soc by more than 5%,
re-init the counter from it. The pair is a true open circuit voltage,
so this corrects a counter that is wrong by any amount, and it is the
mechanism the BSP driver uses at resume.

Expose PWRON_VOL as POWER_SUPPLY_PROP_VOLTAGE_BOOT and the current
rested pair as POWER_SUPPLY_PROP_VOLTAGE_OCV, -ENODATA while the
battery is not resting.

Log the counter, saved soc and battery voltage at boot, as the BSP logs
its saved and present capacity, so a counter that diverged from the
saved soc is visible.

Fixes: 11cb8da0189b ("power: supply: Add charger driver for Rockchip RK817")
Suggested-by: Jacob Cook <jacobmcook1984@gmail.com>
Co-developed-by: Jacob Cook <jacobmcook1984@gmail.com>
Signed-off-by: Jacob Cook <jacobmcook1984@gmail.com>
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Assisted-by: Claude:claude-5-fable
---
 drivers/power/supply/rk817_charger.c | 222 ++++++++++++++++++++-------
 include/linux/mfd/rk808.h            |   2 +
 2 files changed, 171 insertions(+), 53 deletions(-)

diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
index 86da0462fb59..50fbea298127 100644
--- a/drivers/power/supply/rk817_charger.c
+++ b/drivers/power/supply/rk817_charger.c
@@ -95,6 +95,15 @@ struct rk817_charger {
 	int bat_voltage_min_design_uv;
 	int bat_voltage_max_design_uv;
 
+	/* Battery voltage the PMIC sampled when the system powered on. */
+	int boot_voltage_uv;
+
+	/* Rested voltage the gauge latched, 0 if it took no usable pair. */
+	int relax_voltage_uv;
+
+	/* Retained for the ocv-capacity table the recalibration needs. */
+	struct power_supply_battery_info *bat_info;
+
 	/* Values updated periodically by driver for display. */
 	int charge_now_uah;
 	int volt_avg_uv;
@@ -332,6 +341,42 @@ static int rk817_bat_calib_cap(struct rk817_charger *charger)
 	return 0;
 }
 
+/* Calibrated microvolts from one of the gas gauge's 16 bit voltage registers. */
+static int rk817_bat_voltage_uv(struct rk817_charger *charger, unsigned int reg)
+{
+	u8 bulk_reg[2];
+
+	regmap_bulk_read(charger->rk808->regmap, reg, bulk_reg, 2);
+
+	return (charger->voltage_k * get_unaligned_be16(bulk_reg)) +
+		1000 * charger->voltage_b;
+}
+
+/*
+ * Voltage the gauge latched while the battery was resting. Relax mode only
+ * samples below RELAX_THRE_CUR, so this is a true open circuit voltage with
+ * no resistance term. Returns 0 unless the battery is resting now and the
+ * latched pair is inside the pack's design range.
+ */
+static int rk817_bat_relax_voltage_uv(struct rk817_charger *charger)
+{
+	unsigned int need = RK817_RELAX_STS | RK817_RELAX_VOL_UPD;
+	int ret, sts, vol;
+
+	ret = regmap_read(charger->rk808->regmap, RK817_GAS_GAUGE_GG_STS, &sts);
+	if (ret < 0 || (sts & need) != need)
+		return 0;
+
+	vol = max(rk817_bat_voltage_uv(charger, RK817_GAS_GAUGE_RELAX_VOL1_H),
+		  rk817_bat_voltage_uv(charger, RK817_GAS_GAUGE_RELAX_VOL2_H));
+
+	if (vol < charger->bat_voltage_min_design_uv ||
+	    vol > charger->bat_voltage_max_design_uv)
+		return 0;
+
+	return vol;
+}
+
 static void rk817_read_props(struct rk817_charger *charger)
 {
 	int tmp, reg;
@@ -367,11 +412,8 @@ static void rk817_read_props(struct rk817_charger *charger)
 	charger->soc = charger->charge_now_uah * 100 / charger->fcc_mah;
 
 	/* Update reported voltage. */
-	regmap_bulk_read(charger->rk808->regmap, RK817_GAS_GAUGE_BAT_VOL_H,
-			 bulk_reg, 2);
-	tmp = get_unaligned_be16(bulk_reg);
-	charger->volt_avg_uv = (charger->voltage_k * tmp) + 1000 *
-				charger->voltage_b;
+	charger->volt_avg_uv = rk817_bat_voltage_uv(charger,
+						    RK817_GAS_GAUGE_BAT_VOL_H);
 
 	/*
 	 * Update reported current. Note value from registers is a signed 16
@@ -430,6 +472,8 @@ static void rk817_read_props(struct rk817_charger *charger)
 		charger->charger_input_volt_avg_uv = 0;
 	}
 
+	charger->relax_voltage_uv = rk817_bat_relax_voltage_uv(charger);
+
 	/* Calibrate battery capacity and soc. */
 	rk817_bat_calib_cap(charger);
 }
@@ -516,6 +560,14 @@ static int rk817_bat_get_prop(struct power_supply *ps,
 	case POWER_SUPPLY_PROP_VOLTAGE_AVG:
 		val->intval = charger->volt_avg_uv;
 		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_BOOT:
+		val->intval = charger->boot_voltage_uv;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_OCV:
+		if (!charger->relax_voltage_uv)
+			return -ENODATA;
+		val->intval = charger->relax_voltage_uv;
+		break;
 	case POWER_SUPPLY_PROP_CURRENT_AVG:
 		val->intval = charger->cur_avg_ua;
 		break;
@@ -654,6 +706,8 @@ static enum power_supply_property rk817_bat_props[] = {
 	POWER_SUPPLY_PROP_CHARGE_NOW,
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
 	POWER_SUPPLY_PROP_VOLTAGE_AVG,
+	POWER_SUPPLY_PROP_VOLTAGE_BOOT,
+	POWER_SUPPLY_PROP_VOLTAGE_OCV,
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
 	POWER_SUPPLY_PROP_CURRENT_AVG,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
@@ -746,15 +800,50 @@ static int rk817_read_battery_nvram_values(struct rk817_charger *charger)
 	return 0;
 }
 
+/*
+ * The relax pair is a true rested voltage: where it disagrees with the
+ * tracked soc by more than 5% (chosen arbitrarily), re-init the counter
+ * from it.
+ */
+static void rk817_bat_ocv_recalibrate(struct rk817_charger *charger)
+{
+	struct rk808 *rk808 = charger->rk808;
+	int ocv_soc, charge_now_adc;
+	u8 bulk_reg[4];
+
+	if (!charger->battery_present)
+		return;
+
+	if (!charger->relax_voltage_uv)
+		return;
+
+	ocv_soc = power_supply_batinfo_ocv2cap(charger->bat_info,
+					       charger->relax_voltage_uv,
+					       20) * 1000;
+	if (ocv_soc < 0 || abs(ocv_soc - charger->soc) <= 5000)
+		return;
+
+	dev_dbg(charger->dev,
+		"OCV %duV disagrees with soc, recalibrating (%d -> %d)\n",
+		charger->relax_voltage_uv, charger->soc, ocv_soc);
+	charger->soc = ocv_soc;
+	charge_now_adc = CHARGE_TO_ADC(ocv_soc * charger->fcc_mah / 100000,
+				       charger->res_div);
+	put_unaligned_be32(charge_now_adc, bulk_reg);
+	regmap_bulk_write(rk808->regmap, RK817_GAS_GAUGE_Q_INIT_H3,
+			  bulk_reg, 4);
+	rk817_record_battery_nvram_values(charger);
+}
+
 static int
 rk817_read_or_set_full_charge_on_boot(struct rk817_charger *charger,
 				struct power_supply_battery_info *bat_info)
 {
 	struct rk808 *rk808 = charger->rk808;
+	u32 boot_charge_mah;
+	int ret, reg, tmp, off_cnt, bat_uv, boot_soc, table_soc;
+	bool first_boot, keep_saved;
 	u8 bulk_reg[4];
-	u32 boot_voltage, boot_charge_mah;
-	int ret, reg, off_time, tmp;
-	bool first_boot;
 
 	/*
 	 * Check if the battery is uninitalized. If it is, the columb counter
@@ -764,25 +853,22 @@ rk817_read_or_set_full_charge_on_boot(struct rk817_charger *charger,
 	if (ret < 0)
 		return ret;
 	first_boot = reg & RK817_BAT_CON;
-	/*
-	 * If the battery is uninitialized, use the poweron voltage and an ocv
-	 * lookup to guess our charge. The number won't be very accurate until
-	 * we hit either our minimum voltage (0%) or full charge (100%).
-	 */
+
+	/* PWRON_VOL only refreshes on a real power-on; bound it to the pack. */
+	charger->boot_voltage_uv =
+		clamp(rk817_bat_voltage_uv(charger, RK817_GAS_GAUGE_PWRON_VOL_H),
+		      bat_info->voltage_min_design_uv,
+		      bat_info->voltage_max_design_uv);
+
 	if (first_boot) {
-		regmap_bulk_read(rk808->regmap, RK817_GAS_GAUGE_PWRON_VOL_H,
-				 bulk_reg, 2);
-		tmp = get_unaligned_be16(bulk_reg);
-		boot_voltage = (charger->voltage_k * tmp) +
-				1000 * charger->voltage_b;
 		/*
-		 * Since only implementation has no working thermistor, assume
-		 * 20C for OCV lookup. If lookup fails, report error with OCV
-		 * table.
+		 * Nothing has been saved yet, so estimate from the boot
+		 * voltage with an ocv lookup. Since only implementation has
+		 * no working thermistor, assume 20C for that lookup.
 		 */
 		charger->soc = power_supply_batinfo_ocv2cap(bat_info,
-							    boot_voltage,
-							    20) * 1000;
+						charger->boot_voltage_uv,
+						20) * 1000;
 		if (charger->soc < 0)
 			charger->soc = 0;
 
@@ -803,39 +889,54 @@ rk817_read_or_set_full_charge_on_boot(struct rk817_charger *charger,
 		if (ret < 0)
 			return ret;
 
+		/*
+		 * The coulomb counter keeps counting while the system is off,
+		 * so take it, capped at full, unless it reads invalid; the
+		 * saved soc is then at most one update interval old. A counter
+		 * that is merely wrong is corrected by the next rested relax
+		 * pair.
+		 */
 		regmap_bulk_read(rk808->regmap, RK817_GAS_GAUGE_Q_PRES_H3,
 				 bulk_reg, 4);
 		tmp = get_unaligned_be32(bulk_reg);
-		if (tmp < 0)
-			tmp = 0;
-		boot_charge_mah = ADC_TO_CHARGE_UAH(tmp,
-						    charger->res_div) / 1000;
-		/*
-		 * Check if the columb counter has been off for more than 30
-		 * minutes as it tends to drift downward. If so, re-init soc
-		 * with the boot voltage instead. Note the unit values for the
-		 * OFF_CNT register appear to be in decaminutes and stops
-		 * counting at 2550 (0xFF) minutes. BSP kernel used OCV, but
-		 * for me occasionally that would show invalid values. Boot
-		 * voltage is only accurate for me on first poweron (not
-		 * reboots), but we shouldn't ever encounter an OFF_CNT more
-		 * than 0 on a reboot anyway.
-		 */
-		regmap_read(rk808->regmap, RK817_GAS_GAUGE_OFF_CNT, &off_time);
-		if (off_time >= 3) {
-			regmap_bulk_read(rk808->regmap,
-					 RK817_GAS_GAUGE_PWRON_VOL_H,
-					 bulk_reg, 2);
-			tmp = get_unaligned_be16(bulk_reg);
-			boot_voltage = (charger->voltage_k * tmp) +
-					1000 * charger->voltage_b;
-			charger->soc =
-				power_supply_batinfo_ocv2cap(bat_info,
-							     boot_voltage,
-							     20) * 1000;
+		regmap_read(rk808->regmap, RK817_GAS_GAUGE_OFF_CNT, &off_cnt);
+		bat_uv = rk817_bat_voltage_uv(charger, RK817_GAS_GAUGE_BAT_VOL_H);
+		dev_dbg(charger->dev,
+			"boot: counter %d uAh, saved soc %d, fcc %d mAh, battery %d uV, off_cnt %d\n",
+			tmp > 0 ? ADC_TO_CHARGE_UAH(tmp, charger->res_div) : 0,
+			charger->soc, charger->fcc_mah, bat_uv, off_cnt);
+		if (tmp > 0) {
+			boot_charge_mah = ADC_TO_CHARGE_UAH(tmp,
+							    charger->res_div) / 1000;
+			if (boot_charge_mah > charger->fcc_mah)
+				boot_charge_mah = charger->fcc_mah;
+			boot_soc = boot_charge_mah * 100000 / charger->fcc_mah;
+
+			/*
+			 * The gauge re-derives this count from its own ocv
+			 * model while the pack rests, so bound it with the ocv
+			 * table: under load the terminal voltage sits below the
+			 * open circuit voltage, making the table a floor, and
+			 * under charge above it, making it a ceiling on any
+			 * saved soc that would displace the counter.
+			 */
+			table_soc = power_supply_batinfo_ocv2cap(bat_info, bat_uv,
+								 20) * 1000;
+			if (charger->plugged_in)
+				keep_saved = boot_soc < charger->soc - 10000 &&
+					     charger->soc <= table_soc + 10000;
+			else
+				keep_saved = boot_soc < table_soc - 10000;
+
+			if (keep_saved)
+				dev_dbg(charger->dev,
+					"counter %d, voltage allows %d, keeping saved soc %d\n",
+					boot_soc, table_soc, charger->soc);
+			else
+				charger->soc = boot_soc;
 		} else {
-			charger->soc = (boot_charge_mah * 1000 * 100 /
-					charger->fcc_mah);
+			dev_dbg(charger->dev,
+				"coulomb counter invalid, keeping saved soc\n");
 		}
 	}
 
@@ -1044,6 +1145,7 @@ static void rk817_charging_monitor(struct work_struct *work)
 	charger = container_of(work, struct rk817_charger, work.work);
 
 	rk817_read_props(charger);
+	rk817_bat_ocv_recalibrate(charger);
 
 	/* Run every 8 seconds like the BSP driver did. */
 	queue_delayed_work(system_percpu_wq, &charger->work, msecs_to_jiffies(8000));
@@ -1056,6 +1158,13 @@ static void rk817_cleanup_node(void *data)
 	of_node_put(node);
 }
 
+static void rk817_put_battery_info(void *data)
+{
+	struct rk817_charger *charger = data;
+
+	power_supply_put_battery_info(charger->bat_ps, charger->bat_info);
+}
+
 static int rk817_charger_probe(struct platform_device *pdev)
 {
 	struct rk808 *rk808 = dev_get_drvdata(pdev->dev.parent);
@@ -1172,7 +1281,14 @@ static int rk817_charger_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	power_supply_put_battery_info(charger->bat_ps, bat_info);
+	/*
+	 * Keep the battery info: the periodic recalibration needs its
+	 * ocv-capacity table for as long as the driver is bound.
+	 */
+	charger->bat_info = bat_info;
+	ret = devm_add_action_or_reset(dev, rk817_put_battery_info, charger);
+	if (ret)
+		return ret;
 
 	plugin_irq = platform_get_irq(pdev, 0);
 	if (plugin_irq < 0)
diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
index 7ffc904c864c..72419d311626 100644
--- a/include/linux/mfd/rk808.h
+++ b/include/linux/mfd/rk808.h
@@ -1211,6 +1211,8 @@ enum rk809_reg_id {
 
 #define RK817_GAS_GAUGE_RELAX_THRE_H	0x58
 #define RK817_GAS_GAUGE_RELAX_THRE_L	0x59
+#define RK817_GAS_GAUGE_RELAX_VOL1_H	0x5a
+#define RK817_GAS_GAUGE_RELAX_VOL2_H	0x5c
 #define RK817_GAS_GAUGE_OCV_THRE_VOL	0x62
 #define RK817_GAS_GAUGE_OCV_VOL_H	0x63
 #define RK817_GAS_GAUGE_OCV_VOL_L	0x64
-- 
2.53.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 3/5] power: supply: rk817-charger: Correct the gauge across system sleep

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

The coulomb counter is erratic while the system sleeps. On an RG353M
charging at 1.4A, one 15 minute sleep registered 69% of the metered
charge and another 139%, and a charge that finished during sleep still
showed the pre-sleep percentage at wake. The signals that could correct
it are gone by the time the driver runs again: RELAX_STS clears as soon
as the wakeup load exceeds the relax threshold (300mA here, less than
an idle UI draws), and CHARGE_FINISH disappears while the charger
re-detects its input. The BSP driver corrects at resume from sleep time
times current and from the relax voltage. Do the same from evidence
that persists:

- At suspend, stamp boottime, charge, current and charging state, and
  clear the relax-updated flags.

- At resume, recompute the soc from the counter, capped at full. A
  relax pair with its flags set was latched during this sleep:
  recalibrate from it without demanding a live RELAX_STS. Otherwise,
  if the system slept charging, credit elapsed * current where the
  counter fell short, saturating at full, which also covers a charge
  that finished during sleep.

The constant-current estimate reads high across a CV taper; the cap
bounds it and the next rested pair corrects the remainder.

Log the counter at suspend and resume, the battery voltage and the
relax pair, so the path taken can be read from the log.

Suggested-by: Jacob Cook <jacobmcook1984@gmail.com>
Co-developed-by: Jacob Cook <jacobmcook1984@gmail.com>
Signed-off-by: Jacob Cook <jacobmcook1984@gmail.com>
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Assisted-by: Claude:claude-5-fable
---
 drivers/power/supply/rk817_charger.c | 109 +++++++++++++++++++++++++--
 1 file changed, 104 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
index 50fbea298127..b6714d8e4fed 100644
--- a/drivers/power/supply/rk817_charger.c
+++ b/drivers/power/supply/rk817_charger.c
@@ -10,6 +10,7 @@
 
 #include <linux/unaligned.h>
 #include <linux/devm-helpers.h>
+#include <linux/math64.h>
 #include <linux/mfd/rk808.h>
 #include <linux/irq.h>
 #include <linux/of.h>
@@ -104,6 +105,12 @@ struct rk817_charger {
 	/* Retained for the ocv-capacity table the recalibration needs. */
 	struct power_supply_battery_info *bat_info;
 
+	/* Stamped at system suspend to correct the gauge across sleep. */
+	time64_t slp_boottime;
+	int slp_charge_uah;
+	int slp_cur_ua;
+	bool slp_charging;
+
 	/* Values updated periodically by driver for display. */
 	int charge_now_uah;
 	int volt_avg_uv;
@@ -355,14 +362,20 @@ static int rk817_bat_voltage_uv(struct rk817_charger *charger, unsigned int reg)
 /*
  * Voltage the gauge latched while the battery was resting. Relax mode only
  * samples below RELAX_THRE_CUR, so this is a true open circuit voltage with
- * no resistance term. Returns 0 unless the battery is resting now and the
- * latched pair is inside the pack's design range.
+ * no resistance term. The update flags are cleared at init and at suspend,
+ * so a pair with them set postdates the last of those; live_only also
+ * requires the battery to be resting now. Returns 0 if there is no usable
+ * pair inside the pack's design range.
  */
-static int rk817_bat_relax_voltage_uv(struct rk817_charger *charger)
+static int rk817_bat_relax_voltage_uv(struct rk817_charger *charger,
+				      bool live_only)
 {
-	unsigned int need = RK817_RELAX_STS | RK817_RELAX_VOL_UPD;
+	unsigned int need = RK817_RELAX_VOL_UPD;
 	int ret, sts, vol;
 
+	if (live_only)
+		need |= RK817_RELAX_STS;
+
 	ret = regmap_read(charger->rk808->regmap, RK817_GAS_GAUGE_GG_STS, &sts);
 	if (ret < 0 || (sts & need) != need)
 		return 0;
@@ -472,7 +485,7 @@ static void rk817_read_props(struct rk817_charger *charger)
 		charger->charger_input_volt_avg_uv = 0;
 	}
 
-	charger->relax_voltage_uv = rk817_bat_relax_voltage_uv(charger);
+	charger->relax_voltage_uv = rk817_bat_relax_voltage_uv(charger, true);
 
 	/* Calibrate battery capacity and soc. */
 	rk817_bat_calib_cap(charger);
@@ -1323,12 +1336,96 @@ static int rk817_charger_probe(struct platform_device *pdev)
 	return 0;
 }
 
+/*
+ * The coulomb counter is erratic while the system sleeps, and the signals
+ * that could correct it (a live relax status, a finished charge) are gone
+ * before the monitor runs again. Correct from what persists instead, as
+ * the BSP driver does.
+ */
+static void rk817_bat_sleep_adjust(struct rk817_charger *charger)
+{
+	struct rk808 *rk808 = charger->rk808;
+	time64_t elapsed = ktime_get_boottime_seconds() - charger->slp_boottime;
+	int expected_uah, moved_uah, charge_uah, credit_uah, tmp;
+	unsigned int sts;
+	u8 bulk_reg[4];
+
+	if (!charger->battery_present || elapsed <= 0)
+		return;
+
+	regmap_read(rk808->regmap, RK817_SYS_STS, &sts);
+	charger->plugged_in = (sts & RK817_PLUG_IN_STS);
+
+	/* Start from what the counter did during sleep, not the pre-sleep soc. */
+	regmap_bulk_read(rk808->regmap, RK817_GAS_GAUGE_Q_PRES_H3,
+			 bulk_reg, 4);
+	tmp = get_unaligned_be32(bulk_reg);
+	charge_uah = tmp > 0 ? ADC_TO_CHARGE_UAH(tmp, charger->res_div) : 0;
+	if (charge_uah > charger->fcc_mah * 1000)
+		charge_uah = charger->fcc_mah * 1000;
+	charger->soc = div_s64((s64)charge_uah * 100, charger->fcc_mah);
+
+	/* Flags were cleared at suspend: any pair now is from this sleep. */
+	charger->relax_voltage_uv = rk817_bat_relax_voltage_uv(charger, false);
+	dev_dbg(charger->dev,
+		"resume after %llds: counter %d uAh (%d at suspend), battery %d uV, relax %d uV\n",
+		elapsed, charge_uah, charger->slp_charge_uah,
+		rk817_bat_voltage_uv(charger, RK817_GAS_GAUGE_BAT_VOL_H),
+		charger->relax_voltage_uv);
+	if (charger->relax_voltage_uv) {
+		rk817_bat_ocv_recalibrate(charger);
+		return;
+	}
+
+	if (!charger->slp_charging || !charger->plugged_in)
+		return;
+
+	moved_uah = charge_uah - charger->slp_charge_uah;
+	expected_uah = div_s64((s64)charger->slp_cur_ua * elapsed, 3600);
+	credit_uah = expected_uah - moved_uah;
+
+	dev_info(charger->dev,
+		 "slept charging for %llds: counter moved %d of expected %d uAh\n",
+		 (long long)elapsed, moved_uah, expected_uah);
+
+	if (credit_uah <= 0)
+		return;
+
+	/*
+	 * Credit the shortfall, saturating at full - covers a charge that
+	 * terminated during sleep. A constant-current estimate reads high
+	 * across a taper; the cap bounds it, the next rested pair corrects
+	 * the rest.
+	 */
+	charge_uah += credit_uah;
+	if (charge_uah > charger->fcc_mah * 1000)
+		charge_uah = charger->fcc_mah * 1000;
+
+	charger->soc = div_s64((s64)charge_uah * 100, charger->fcc_mah);
+	tmp = CHARGE_TO_ADC(charge_uah / 1000, charger->res_div);
+	put_unaligned_be32(tmp, bulk_reg);
+	regmap_bulk_write(rk808->regmap, RK817_GAS_GAUGE_Q_INIT_H3,
+			  bulk_reg, 4);
+	rk817_record_battery_nvram_values(charger);
+}
+
 static int __maybe_unused rk817_suspend(struct device *dev)
 {
 	struct rk817_charger *charger = dev_get_drvdata(dev);
 
 	cancel_delayed_work_sync(&charger->work);
 
+	charger->slp_boottime = ktime_get_boottime_seconds();
+	charger->slp_charge_uah = charger->charge_now_uah;
+	charger->slp_cur_ua = abs(charger->cur_avg_ua);
+	charger->slp_charging = charger->plugged_in &&
+				(charger->charge_status == CC_OR_CV_CHRG ||
+				 charger->charge_status == TRICKLE_CHRG);
+
+	/* Any relax pair found at resume will postdate this suspend. */
+	regmap_write_bits(charger->rk808->regmap, RK817_GAS_GAUGE_GG_STS,
+			  RK817_RELAX_VOL_UPD, 0);
+
 	return 0;
 }
 
@@ -1337,6 +1434,8 @@ static int __maybe_unused rk817_resume(struct device *dev)
 
 	struct rk817_charger *charger = dev_get_drvdata(dev);
 
+	rk817_bat_sleep_adjust(charger);
+
 	/* force an immediate update */
 	mod_delayed_work(system_percpu_wq, &charger->work, 0);
 
-- 
2.53.0

[PATCH 3/5] power: supply: rk817-charger: Correct the gauge across system sleep

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

The coulomb counter is erratic while the system sleeps. On an RG353M
charging at 1.4A, one 15 minute sleep registered 69% of the metered
charge and another 139%, and a charge that finished during sleep still
showed the pre-sleep percentage at wake. The signals that could correct
it are gone by the time the driver runs again: RELAX_STS clears as soon
as the wakeup load exceeds the relax threshold (300mA here, less than
an idle UI draws), and CHARGE_FINISH disappears while the charger
re-detects its input. The BSP driver corrects at resume from sleep time
times current and from the relax voltage. Do the same from evidence
that persists:

- At suspend, stamp boottime, charge, current and charging state, and
  clear the relax-updated flags.

- At resume, recompute the soc from the counter, capped at full. A
  relax pair with its flags set was latched during this sleep:
  recalibrate from it without demanding a live RELAX_STS. Otherwise,
  if the system slept charging, credit elapsed * current where the
  counter fell short, saturating at full, which also covers a charge
  that finished during sleep.

The constant-current estimate reads high across a CV taper; the cap
bounds it and the next rested pair corrects the remainder.

Log the counter at suspend and resume, the battery voltage and the
relax pair, so the path taken can be read from the log.

Suggested-by: Jacob Cook <jacobmcook1984@gmail.com>
Co-developed-by: Jacob Cook <jacobmcook1984@gmail.com>
Signed-off-by: Jacob Cook <jacobmcook1984@gmail.com>
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Assisted-by: Claude:claude-5-fable
---
 drivers/power/supply/rk817_charger.c | 109 +++++++++++++++++++++++++--
 1 file changed, 104 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
index 50fbea298127..b6714d8e4fed 100644
--- a/drivers/power/supply/rk817_charger.c
+++ b/drivers/power/supply/rk817_charger.c
@@ -10,6 +10,7 @@
 
 #include <linux/unaligned.h>
 #include <linux/devm-helpers.h>
+#include <linux/math64.h>
 #include <linux/mfd/rk808.h>
 #include <linux/irq.h>
 #include <linux/of.h>
@@ -104,6 +105,12 @@ struct rk817_charger {
 	/* Retained for the ocv-capacity table the recalibration needs. */
 	struct power_supply_battery_info *bat_info;
 
+	/* Stamped at system suspend to correct the gauge across sleep. */
+	time64_t slp_boottime;
+	int slp_charge_uah;
+	int slp_cur_ua;
+	bool slp_charging;
+
 	/* Values updated periodically by driver for display. */
 	int charge_now_uah;
 	int volt_avg_uv;
@@ -355,14 +362,20 @@ static int rk817_bat_voltage_uv(struct rk817_charger *charger, unsigned int reg)
 /*
  * Voltage the gauge latched while the battery was resting. Relax mode only
  * samples below RELAX_THRE_CUR, so this is a true open circuit voltage with
- * no resistance term. Returns 0 unless the battery is resting now and the
- * latched pair is inside the pack's design range.
+ * no resistance term. The update flags are cleared at init and at suspend,
+ * so a pair with them set postdates the last of those; live_only also
+ * requires the battery to be resting now. Returns 0 if there is no usable
+ * pair inside the pack's design range.
  */
-static int rk817_bat_relax_voltage_uv(struct rk817_charger *charger)
+static int rk817_bat_relax_voltage_uv(struct rk817_charger *charger,
+				      bool live_only)
 {
-	unsigned int need = RK817_RELAX_STS | RK817_RELAX_VOL_UPD;
+	unsigned int need = RK817_RELAX_VOL_UPD;
 	int ret, sts, vol;
 
+	if (live_only)
+		need |= RK817_RELAX_STS;
+
 	ret = regmap_read(charger->rk808->regmap, RK817_GAS_GAUGE_GG_STS, &sts);
 	if (ret < 0 || (sts & need) != need)
 		return 0;
@@ -472,7 +485,7 @@ static void rk817_read_props(struct rk817_charger *charger)
 		charger->charger_input_volt_avg_uv = 0;
 	}
 
-	charger->relax_voltage_uv = rk817_bat_relax_voltage_uv(charger);
+	charger->relax_voltage_uv = rk817_bat_relax_voltage_uv(charger, true);
 
 	/* Calibrate battery capacity and soc. */
 	rk817_bat_calib_cap(charger);
@@ -1323,12 +1336,96 @@ static int rk817_charger_probe(struct platform_device *pdev)
 	return 0;
 }
 
+/*
+ * The coulomb counter is erratic while the system sleeps, and the signals
+ * that could correct it (a live relax status, a finished charge) are gone
+ * before the monitor runs again. Correct from what persists instead, as
+ * the BSP driver does.
+ */
+static void rk817_bat_sleep_adjust(struct rk817_charger *charger)
+{
+	struct rk808 *rk808 = charger->rk808;
+	time64_t elapsed = ktime_get_boottime_seconds() - charger->slp_boottime;
+	int expected_uah, moved_uah, charge_uah, credit_uah, tmp;
+	unsigned int sts;
+	u8 bulk_reg[4];
+
+	if (!charger->battery_present || elapsed <= 0)
+		return;
+
+	regmap_read(rk808->regmap, RK817_SYS_STS, &sts);
+	charger->plugged_in = (sts & RK817_PLUG_IN_STS);
+
+	/* Start from what the counter did during sleep, not the pre-sleep soc. */
+	regmap_bulk_read(rk808->regmap, RK817_GAS_GAUGE_Q_PRES_H3,
+			 bulk_reg, 4);
+	tmp = get_unaligned_be32(bulk_reg);
+	charge_uah = tmp > 0 ? ADC_TO_CHARGE_UAH(tmp, charger->res_div) : 0;
+	if (charge_uah > charger->fcc_mah * 1000)
+		charge_uah = charger->fcc_mah * 1000;
+	charger->soc = div_s64((s64)charge_uah * 100, charger->fcc_mah);
+
+	/* Flags were cleared at suspend: any pair now is from this sleep. */
+	charger->relax_voltage_uv = rk817_bat_relax_voltage_uv(charger, false);
+	dev_dbg(charger->dev,
+		"resume after %llds: counter %d uAh (%d at suspend), battery %d uV, relax %d uV\n",
+		elapsed, charge_uah, charger->slp_charge_uah,
+		rk817_bat_voltage_uv(charger, RK817_GAS_GAUGE_BAT_VOL_H),
+		charger->relax_voltage_uv);
+	if (charger->relax_voltage_uv) {
+		rk817_bat_ocv_recalibrate(charger);
+		return;
+	}
+
+	if (!charger->slp_charging || !charger->plugged_in)
+		return;
+
+	moved_uah = charge_uah - charger->slp_charge_uah;
+	expected_uah = div_s64((s64)charger->slp_cur_ua * elapsed, 3600);
+	credit_uah = expected_uah - moved_uah;
+
+	dev_info(charger->dev,
+		 "slept charging for %llds: counter moved %d of expected %d uAh\n",
+		 (long long)elapsed, moved_uah, expected_uah);
+
+	if (credit_uah <= 0)
+		return;
+
+	/*
+	 * Credit the shortfall, saturating at full - covers a charge that
+	 * terminated during sleep. A constant-current estimate reads high
+	 * across a taper; the cap bounds it, the next rested pair corrects
+	 * the rest.
+	 */
+	charge_uah += credit_uah;
+	if (charge_uah > charger->fcc_mah * 1000)
+		charge_uah = charger->fcc_mah * 1000;
+
+	charger->soc = div_s64((s64)charge_uah * 100, charger->fcc_mah);
+	tmp = CHARGE_TO_ADC(charge_uah / 1000, charger->res_div);
+	put_unaligned_be32(tmp, bulk_reg);
+	regmap_bulk_write(rk808->regmap, RK817_GAS_GAUGE_Q_INIT_H3,
+			  bulk_reg, 4);
+	rk817_record_battery_nvram_values(charger);
+}
+
 static int __maybe_unused rk817_suspend(struct device *dev)
 {
 	struct rk817_charger *charger = dev_get_drvdata(dev);
 
 	cancel_delayed_work_sync(&charger->work);
 
+	charger->slp_boottime = ktime_get_boottime_seconds();
+	charger->slp_charge_uah = charger->charge_now_uah;
+	charger->slp_cur_ua = abs(charger->cur_avg_ua);
+	charger->slp_charging = charger->plugged_in &&
+				(charger->charge_status == CC_OR_CV_CHRG ||
+				 charger->charge_status == TRICKLE_CHRG);
+
+	/* Any relax pair found at resume will postdate this suspend. */
+	regmap_write_bits(charger->rk808->regmap, RK817_GAS_GAUGE_GG_STS,
+			  RK817_RELAX_VOL_UPD, 0);
+
 	return 0;
 }
 
@@ -1337,6 +1434,8 @@ static int __maybe_unused rk817_resume(struct device *dev)
 
 	struct rk817_charger *charger = dev_get_drvdata(dev);
 
+	rk817_bat_sleep_adjust(charger);
+
 	/* force an immediate update */
 	mod_delayed_work(system_percpu_wq, &charger->work, 0);
 
-- 
2.53.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add a boolean attribute for the rk817-charger to control whether or not
the system power is shutdown in battery off mode. The BSP kernel sets
this value unless a device-tree attribute is present, and this attribute
is to replicate that behavior.

Fixes: a592aa3a2a8c ("dt-bindings: mfd: Add Rockchip rk817 battery charger support")
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Suggested-by: Zetarancio <zetarancio@users.noreply.github.com>
---
 Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
index 2cb6d176a84c..4cf987fe763b 100644
--- a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
+++ b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
@@ -148,6 +148,12 @@ properties:
           Value in microamps of the sleep filter current for the charger.
           Value is used by the driver to derive the sleep sample current.
 
+      rockchip,gate-function-disable:
+        type: boolean
+        description:
+          Boolean value to prevent PMIC from turning off system voltage when
+          battery voltage is shut down.
+
     required:
       - monitored-battery
       - rockchip,resistor-sense-micro-ohms
@@ -381,6 +387,7 @@ examples:
 
             rk817_charger: charger {
                 monitored-battery = <&battery>;
+                rockchip,gate-function-disable;
                 rockchip,resistor-sense-micro-ohms = <10000>;
                 rockchip,sleep-enter-current-microamp = <300000>;
                 rockchip,sleep-filter-current-microamp = <100000>;
-- 
2.53.0

[PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add a boolean attribute for the rk817-charger to control whether or not
the system power is shutdown in battery off mode. The BSP kernel sets
this value unless a device-tree attribute is present, and this attribute
is to replicate that behavior.

Fixes: a592aa3a2a8c ("dt-bindings: mfd: Add Rockchip rk817 battery charger support")
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Suggested-by: Zetarancio <zetarancio@users.noreply.github.com>
---
 Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
index 2cb6d176a84c..4cf987fe763b 100644
--- a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
+++ b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
@@ -148,6 +148,12 @@ properties:
           Value in microamps of the sleep filter current for the charger.
           Value is used by the driver to derive the sleep sample current.
 
+      rockchip,gate-function-disable:
+        type: boolean
+        description:
+          Boolean value to prevent PMIC from turning off system voltage when
+          battery voltage is shut down.
+
     required:
       - monitored-battery
       - rockchip,resistor-sense-micro-ohms
@@ -381,6 +387,7 @@ examples:
 
             rk817_charger: charger {
                 monitored-battery = <&battery>;
+                rockchip,gate-function-disable;
                 rockchip,resistor-sense-micro-ohms = <10000>;
                 rockchip,sleep-enter-current-microamp = <300000>;
                 rockchip,sleep-filter-current-microamp = <100000>;
-- 
2.53.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 5/5] power: supply: rk817-charger: Clear SYS_CAN_SD register by default

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

The BSP driver for the rk817 charger by default sets the SYS_CAN_SD
register unless a specific attribute is set in the device tree. This
register tells the PMIC that the system voltage can be shut down in
battery off mode.

Though the BSP sets this value by default, it also has a device tree
value to override it.

When this value is not set, it can result in a persistent power
drain while the device is off, as observed by users.

Fixes: 11cb8da0189b ("power: supply: Add charger driver for Rockchip RK817")
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Suggested-by: Zetarancio <zetarancio@users.noreply.github.com>
---
 drivers/power/supply/rk817_charger.c | 9 +++++++++
 include/linux/mfd/rk808.h            | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
index b6714d8e4fed..6971f572c68d 100644
--- a/drivers/power/supply/rk817_charger.c
+++ b/drivers/power/supply/rk817_charger.c
@@ -1148,6 +1148,15 @@ static int rk817_battery_init(struct rk817_charger *charger,
 	regmap_write_bits(rk808->regmap, RK817_PMIC_CHRG_IN, RK817_USB_ILIM_EN,
 			  (0x01 << 3));
 
+	/*
+	 * Shutdown the system voltage in bat_off mode, unless
+	 * gate-function-disable is set.
+	 */
+	if (!device_property_read_bool(charger->dev,
+				       "rockchip,gate-function-disable"))
+		regmap_clear_bits(rk808->regmap,
+				  RK817_PMIC_CHRG_TERM, RK817_SYS_CAN_SD);
+
 	return 0;
 }
 
diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
index 72419d311626..2bc58de53d82 100644
--- a/include/linux/mfd/rk808.h
+++ b/include/linux/mfd/rk808.h
@@ -1297,6 +1297,7 @@ enum rk809_reg_id {
 #define RK817_USB_ILIM_EN		(0x01 << 3)
 #define RK817_USB_ILIM_SEL		(0x07 << 0)
 #define RK817_PMIC_CHRG_TERM		0xe6
+#define RK817_SYS_CAN_SD		(0x01 << 7)
 #define RK817_CHRG_TERM_ANA_DIG		(0x01 << 2)
 #define RK817_CHRG_TERM_ANA_SEL		(0x03 << 0)
 #define RK817_CHRG_EN			(0x01 << 6)
-- 
2.53.0

[PATCH 5/5] power: supply: rk817-charger: Clear SYS_CAN_SD register by default

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

The BSP driver for the rk817 charger by default sets the SYS_CAN_SD
register unless a specific attribute is set in the device tree. This
register tells the PMIC that the system voltage can be shut down in
battery off mode.

Though the BSP sets this value by default, it also has a device tree
value to override it.

When this value is not set, it can result in a persistent power
drain while the device is off, as observed by users.

Fixes: 11cb8da0189b ("power: supply: Add charger driver for Rockchip RK817")
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Suggested-by: Zetarancio <zetarancio@users.noreply.github.com>
---
 drivers/power/supply/rk817_charger.c | 9 +++++++++
 include/linux/mfd/rk808.h            | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
index b6714d8e4fed..6971f572c68d 100644
--- a/drivers/power/supply/rk817_charger.c
+++ b/drivers/power/supply/rk817_charger.c
@@ -1148,6 +1148,15 @@ static int rk817_battery_init(struct rk817_charger *charger,
 	regmap_write_bits(rk808->regmap, RK817_PMIC_CHRG_IN, RK817_USB_ILIM_EN,
 			  (0x01 << 3));
 
+	/*
+	 * Shutdown the system voltage in bat_off mode, unless
+	 * gate-function-disable is set.
+	 */
+	if (!device_property_read_bool(charger->dev,
+				       "rockchip,gate-function-disable"))
+		regmap_clear_bits(rk808->regmap,
+				  RK817_PMIC_CHRG_TERM, RK817_SYS_CAN_SD);
+
 	return 0;
 }
 
diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
index 72419d311626..2bc58de53d82 100644
--- a/include/linux/mfd/rk808.h
+++ b/include/linux/mfd/rk808.h
@@ -1297,6 +1297,7 @@ enum rk809_reg_id {
 #define RK817_USB_ILIM_EN		(0x01 << 3)
 #define RK817_USB_ILIM_SEL		(0x07 << 0)
 #define RK817_PMIC_CHRG_TERM		0xe6
+#define RK817_SYS_CAN_SD		(0x01 << 7)
 #define RK817_CHRG_TERM_ANA_DIG		(0x01 << 2)
 #define RK817_CHRG_TERM_ANA_SEL		(0x03 << 0)
 #define RK817_CHRG_EN			(0x01 << 6)
-- 
2.53.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 5/5] power: supply: rk817-charger: Clear SYS_CAN_SD register by default

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] power: supply: rk817-charger: property read on node-less charger->dev in rk817_battery_init()
- [High] power: supply: rk817-charger: gate-function-disable read from NULL dev fwnode
--

commit 925404edbf883b384fffee5ff2dad1b5fbad4340
Author: Chris Morgan <macromorgan@hotmail.com>

power: supply: rk817-charger: Clear SYS_CAN_SD register by default

This commit clears the RK817_SYS_CAN_SD bit in the RK817_PMIC_CHRG_TERM
register during battery initialization unless the
rockchip,gate-function-disable property is present, in order to prevent
persistent power drain while the device is powered off.

>     power: supply: rk817-charger: Clear SYS_CAN_SD register by default
>     
>     The BSP driver for the rk817 charger by default sets the SYS_CAN_SD
>     register unless a specific attribute is set in the device tree. This
>     register tells the PMIC that the system voltage can be shut down in
>     battery off mode.
>     
>     Though the BSP sets this value by default, it also has a device tree
>     value to override it.
>     
>     When this value is not set, it can result in a persistent power
>     drain while the device is off, as observed by users.
>     
>     Fixes: 11cb8da0189b ("power: supply: Add charger driver for Rockchip RK817")
>     Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>     Suggested-by: Zetarancio <zetarancio@users.noreply.github.com>

[Severity: Low]
This isn't a bug in the code, but the commit message body says the driver by
default sets the SYS_CAN_SD bit and that power drain occurs when this value
is not set, whereas the commit subject and the code call regmap_clear_bits()
to clear RK817_SYS_CAN_SD. Should the commit message body say "clears"
instead of "sets"?

> diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
> index b6714d8e4fed9..6971f572c68d0 100644
> --- a/drivers/power/supply/rk817_charger.c
> +++ b/drivers/power/supply/rk817_charger.c
> @@ -1148,6 +1148,15 @@ static int rk817_battery_init(struct rk817_charger *charger,
>  	regmap_write_bits(rk808->regmap, RK817_PMIC_CHRG_IN, RK817_USB_ILIM_EN,
>  			  (0x01 << 3));
>  
> +	/*
> +	 * Shutdown the system voltage in bat_off mode, unless
> +	 * gate-function-disable is set.
> +	 */
> +	if (!device_property_read_bool(charger->dev,
> +				       "rockchip,gate-function-disable"))
> +		regmap_clear_bits(rk808->regmap,
> +				  RK817_PMIC_CHRG_TERM, RK817_SYS_CAN_SD);

[Severity: High]
Will device_property_read_bool(charger->dev, "rockchip,gate-function-disable")
ever find the property here?

In drivers/mfd/rk8xx-core.c, the mfd_cell entry in rk817s[] for
rk817-charger does not set .of_compatible (and the charger child node in
Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml has no compatible
property):

drivers/mfd/rk8xx-core.c:
	{
		.name = "rk817-charger",
		.num_resources = ARRAY_SIZE(rk817_charger_resources),
		.resources = &rk817_charger_resources[0],
	},

Because of this, mfd_add_device() leaves pdev->dev.of_node and
pdev->dev.fwnode as NULL.

In rk817_charger_probe(), the driver manually looks up the child node from
the parent PMIC and only attaches its fwnode to pscfg.fwnode for the
registered power_supply devices, while reading all other charger properties
directly from node:

drivers/power/supply/rk817_charger.c:rk817_charger_probe() {
	node = of_get_child_by_name(dev->parent->of_node, "charger");
	if (!node)
		return -ENODEV;
	...
	charger->dev = &pdev->dev;
	platform_set_drvdata(pdev, charger);

	rk817_bat_calib_vol(charger);

	pscfg.drv_data = charger;
	pscfg.fwnode = &node->fwnode;
	...
	ret = of_property_read_u32(node, "rockchip,resistor-sense-micro-ohms",
				   &of_value);
}

Since dev_fwnode(charger->dev) is NULL when rk817_charger_probe() calls
rk817_battery_init(), device_property_read_bool(charger->dev, ...) always
returns false and RK817_SYS_CAN_SD is cleared unconditionally even when
rockchip,gate-function-disable is present in the charger Device Tree node.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026174834.1652309-1-macroalpha82@gmail.com?part=5

Re: [PATCH 2/5] power: supply: rk817-charger: Resolve boot state of charge from the coulomb counter

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] power: supply: rk817-charger: flawed keep_saved check on unplugged boot
--

commit ca12b28c400d82b7a66e329e1e7db82f71128a48
Author: Chris Morgan <macromorgan@hotmail.com>

power: supply: rk817-charger: Resolve boot state of charge from the coulomb counter

This commit resolves the boot state of charge from the coulomb counter on
non-first boots instead of relying on PWRON_VOL when OFF_CNT is three or
more, and adds periodic recalibration against the relaxed open-circuit
voltage pair. It also exposes POWER_SUPPLY_PROP_VOLTAGE_BOOT and
POWER_SUPPLY_PROP_VOLTAGE_OCV.

> diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
> index 86da0462fb594..50fbea298127f 100644
> --- a/drivers/power/supply/rk817_charger.c
> +++ b/drivers/power/supply/rk817_charger.c

[ ... ]

> @@ -803,39 +889,54 @@ rk817_read_or_set_full_charge_on_boot(struct rk817_charger *charger,
>  		if (ret < 0)
>  			return ret;
>  
> +		/*
> +		 * The coulomb counter keeps counting while the system is off,
> +		 * so take it, capped at full, unless it reads invalid; the
> +		 * saved soc is then at most one update interval old. A counter
> +		 * that is merely wrong is corrected by the next rested relax
> +		 * pair.
> +		 */
>  		regmap_bulk_read(rk808->regmap, RK817_GAS_GAUGE_Q_PRES_H3,
>  				 bulk_reg, 4);
>  		tmp = get_unaligned_be32(bulk_reg);

[ ... ]

> +		regmap_read(rk808->regmap, RK817_GAS_GAUGE_OFF_CNT, &off_cnt);
> +		bat_uv = rk817_bat_voltage_uv(charger, RK817_GAS_GAUGE_BAT_VOL_H);
> +		dev_dbg(charger->dev,
> +			"boot: counter %d uAh, saved soc %d, fcc %d mAh, battery %d uV, off_cnt %d\n",
> +			tmp > 0 ? ADC_TO_CHARGE_UAH(tmp, charger->res_div) : 0,
> +			charger->soc, charger->fcc_mah, bat_uv, off_cnt);
> +		if (tmp > 0) {
> +			boot_charge_mah = ADC_TO_CHARGE_UAH(tmp,
> +							    charger->res_div) / 1000;
> +			if (boot_charge_mah > charger->fcc_mah)
> +				boot_charge_mah = charger->fcc_mah;
> +			boot_soc = boot_charge_mah * 100000 / charger->fcc_mah;
> +
> +			/*
> +			 * The gauge re-derives this count from its own ocv
> +			 * model while the pack rests, so bound it with the ocv
> +			 * table: under load the terminal voltage sits below the
> +			 * open circuit voltage, making the table a floor, and
> +			 * under charge above it, making it a ceiling on any
> +			 * saved soc that would displace the counter.
> +			 */
> +			table_soc = power_supply_batinfo_ocv2cap(bat_info, bat_uv,
> +								 20) * 1000;
> +			if (charger->plugged_in)
> +				keep_saved = boot_soc < charger->soc - 10000 &&
> +					     charger->soc <= table_soc + 10000;
> +			else
> +				keep_saved = boot_soc < table_soc - 10000;
> +
> +			if (keep_saved)
> +				dev_dbg(charger->dev,
> +					"counter %d, voltage allows %d, keeping saved soc %d\n",
> +					boot_soc, table_soc, charger->soc);
> +			else
> +				charger->soc = boot_soc;

[Severity: Medium]
In rk817_read_or_set_full_charge_on_boot() in
drivers/power/supply/rk817_charger.c, when !charger->plugged_in, keep_saved
is set to:

	keep_saved = boot_soc < table_soc - 10000;

without comparing charger->soc (from NVRAM) against either boot_soc or
table_soc.

Immediately after this branch, rk817_read_or_set_full_charge_on_boot() writes
charger->soc back into the hardware coulomb counter:

	boot_charge_mah = charger->soc * charger->fcc_mah / 100 / 1000;
	if (boot_charge_mah > charger->fcc_mah)
		boot_charge_mah = charger->fcc_mah;
	tmp = CHARGE_TO_ADC(boot_charge_mah, charger->res_div);
	put_unaligned_be32(tmp, bulk_reg);
	ret = regmap_bulk_write(rk808->regmap, RK817_GAS_GAUGE_Q_INIT_H3,
			  bulk_reg, 4);

If a device is powered off at a low state of charge (for example,
charger->soc = 15000), charged while powered off, and then unplugged before
booting (!charger->plugged_in), and the gauge's internal OCV model re-derives
boot_soc to more than 10% below table_soc (say, table_soc = 80000 and
boot_soc = 65000), keep_saved evaluates to true. That discards boot_soc and
reinitializes RK817_GAS_GAUGE_Q_INIT_H3 with the stale pre-charge NVRAM
charger->soc (15000), which is even further below the table_soc floor.

Conversely, if the device is powered off at a high state of charge (for
example, charger->soc = 95000) and discharges while off to table_soc = 50000
with boot_soc = 38000, keep_saved also evaluates to true and overwrites
RK817_GAS_GAUGE_Q_INIT_H3 with the stale 95000 value.

In addition, both the commit message ("Keep the PWRON_VOL lookup for the
first boot. Otherwise take the counter, capped at full, unless it reads zero
or negative, in which case keep the saved soc...") and the comment above this
block say that a positive coulomb counter is adopted on boot and corrected
later by the next rested relax voltage pair.

Should the !charger->plugged_in case also verify that charger->soc is a
better estimate than boot_soc before keeping the saved value, or should
rk817_read_or_set_full_charge_on_boot() unconditionally use boot_soc when
tmp > 0 as described in the commit message?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026174834.1652309-1-macroalpha82@gmail.com?part=2

Re: [PATCH 3/5] power: supply: rk817-charger: Correct the gauge across system sleep

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] power: supply: rk817-charger: rk817_suspend() treats discharge current as charge
- [Medium] power: supply: rk817-charger: uncalibrated hardware counter in rk817_bat_sleep_adjust()
- [Medium] power: supply: rk817-charger: stale charge_now_uah stamped into slp_charge_uah
--

commit 90ab344fb97258fd37a11933de3725ce00041ea5
Author: Chris Morgan <macromorgan@hotmail.com>

power: supply: rk817-charger: Correct the gauge across system sleep

This commit adjusts the RK817 gas gauge across system sleep by stamping
boottime, charge, current, and charging status at suspend, and then
recalibrating on resume from either a latched relax voltage or an
estimated constant-current charge delta.

> diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
> index 50fbea298127f..b6714d8e4fed9 100644
> --- a/drivers/power/supply/rk817_charger.c
> +++ b/drivers/power/supply/rk817_charger.c

[ ... ]

> @@ -1323,12 +1336,96 @@ static int rk817_charger_probe(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +/*
> + * The coulomb counter is erratic while the system sleeps, and the signals
> + * that could correct it (a live relax status, a finished charge) are gone
> + * before the monitor runs again. Correct from what persists instead, as
> + * the BSP driver does.
> + */
> +static void rk817_bat_sleep_adjust(struct rk817_charger *charger)
> +{
> +	struct rk808 *rk808 = charger->rk808;
> +	time64_t elapsed = ktime_get_boottime_seconds() - charger->slp_boottime;
> +	int expected_uah, moved_uah, charge_uah, credit_uah, tmp;
> +	unsigned int sts;
> +	u8 bulk_reg[4];
> +
> +	if (!charger->battery_present || elapsed <= 0)
> +		return;
> +
> +	regmap_read(rk808->regmap, RK817_SYS_STS, &sts);
> +	charger->plugged_in = (sts & RK817_PLUG_IN_STS);
> +
> +	/* Start from what the counter did during sleep, not the pre-sleep soc. */
> +	regmap_bulk_read(rk808->regmap, RK817_GAS_GAUGE_Q_PRES_H3,
> +			 bulk_reg, 4);
> +	tmp = get_unaligned_be32(bulk_reg);
> +	charge_uah = tmp > 0 ? ADC_TO_CHARGE_UAH(tmp, charger->res_div) : 0;
> +	if (charge_uah > charger->fcc_mah * 1000)
> +		charge_uah = charger->fcc_mah * 1000;
> +	charger->soc = div_s64((s64)charge_uah * 100, charger->fcc_mah);
> +
> +	/* Flags were cleared at suspend: any pair now is from this sleep. */
> +	charger->relax_voltage_uv = rk817_bat_relax_voltage_uv(charger, false);
> +	dev_dbg(charger->dev,
> +		"resume after %llds: counter %d uAh (%d at suspend), battery %d uV, relax %d uV\n",
> +		elapsed, charge_uah, charger->slp_charge_uah,
> +		rk817_bat_voltage_uv(charger, RK817_GAS_GAUGE_BAT_VOL_H),
> +		charger->relax_voltage_uv);
> +	if (charger->relax_voltage_uv) {
> +		rk817_bat_ocv_recalibrate(charger);
> +		return;
> +	}
> +
> +	if (!charger->slp_charging || !charger->plugged_in)
> +		return;
> +
> +	moved_uah = charge_uah - charger->slp_charge_uah;
> +	expected_uah = div_s64((s64)charger->slp_cur_ua * elapsed, 3600);
> +	credit_uah = expected_uah - moved_uah;
> +
> +	dev_info(charger->dev,
> +		 "slept charging for %llds: counter moved %d of expected %d uAh\n",
> +		 (long long)elapsed, moved_uah, expected_uah);
> +
> +	if (credit_uah <= 0)
> +		return;

[Severity: Medium]
What happens to the hardware coulomb counter (RK817_GAS_GAUGE_Q_PRES_H3) when
it overshoots charger->fcc_mah * 1000 or underflows below 0 during sleep?

In rk817_bat_sleep_adjust(), charge_uah and charger->soc are clamped in
software to [0, charger->fcc_mah * 1000] without writing the clamped value
back to RK817_GAS_GAUGE_Q_INIT_H3.

If a relax voltage was latched during sleep (for example, after finishing
charge and resting at 95%-100% OCV, or resting at 0%-5% OCV after counter
underflow), rk817_bat_sleep_adjust() calls rk817_bat_ocv_recalibrate():

drivers/power/supply/rk817_charger.c:rk817_bat_ocv_recalibrate() {
    ...
	ocv_soc = power_supply_batinfo_ocv2cap(charger->bat_info,
					       charger->relax_voltage_uv,
					       20) * 1000;
	if (ocv_soc < 0 || abs(ocv_soc - charger->soc) <= 5000)
		return;
    ...
}

Because charger->soc was already clamped to 100000 (or 0),
abs(ocv_soc - charger->soc) <= 5000 holds and rk817_bat_ocv_recalibrate()
returns without updating RK817_GAS_GAUGE_Q_INIT_H3.

Similarly, if no relax voltage was latched, rk817_bat_sleep_adjust() returns
without writing RK817_GAS_GAUGE_Q_INIT_H3 whenever !charger->slp_charging,
!charger->plugged_in, or credit_uah <= 0 (which happens whenever the counter
over-counted during sleep), and moved_uah = charge_uah - charger->slp_charge_uah
is computed from the clamped charge_uah rather than the actual counter delta.

Since rk817_bat_calib_cap() only caps RK817_GAS_GAUGE_Q_PRES_H3 when
charge_status == CHARGE_FINISH && charger->soc_cal, won't the hardware
accumulator retain the phantom excess above full capacity (or deficit below 0)
and keep the reported SoC pinned after resume?

> +
> +	/*
> +	 * Credit the shortfall, saturating at full - covers a charge that
> +	 * terminated during sleep. A constant-current estimate reads high
> +	 * across a taper; the cap bounds it, the next rested pair corrects
> +	 * the rest.
> +	 */
> +	charge_uah += credit_uah;
> +	if (charge_uah > charger->fcc_mah * 1000)
> +		charge_uah = charger->fcc_mah * 1000;
> +
> +	charger->soc = div_s64((s64)charge_uah * 100, charger->fcc_mah);
> +	tmp = CHARGE_TO_ADC(charge_uah / 1000, charger->res_div);
> +	put_unaligned_be32(tmp, bulk_reg);
> +	regmap_bulk_write(rk808->regmap, RK817_GAS_GAUGE_Q_INIT_H3,
> +			  bulk_reg, 4);
> +	rk817_record_battery_nvram_values(charger);
> +}
> +
>  static int __maybe_unused rk817_suspend(struct device *dev)
>  {
>  	struct rk817_charger *charger = dev_get_drvdata(dev);
>  
>  	cancel_delayed_work_sync(&charger->work);
>  
> +	charger->slp_boottime = ktime_get_boottime_seconds();
> +	charger->slp_charge_uah = charger->charge_now_uah;

[Severity: Medium]
Can charger->slp_charge_uah be stamped with a stale charger->charge_now_uah
value here?

In rk817_charging_monitor(), rk817_read_props() populates
charger->charge_now_uah from RK817_GAS_GAUGE_Q_PRES_H3 before
rk817_bat_calib_cap() and rk817_bat_ocv_recalibrate() run. When either of
those functions rewrites RK817_GAS_GAUGE_Q_INIT_H3, charger->charge_now_uah is
not updated until the next 8-second monitor cycle.

Likewise, in rk817_resume(), rk817_bat_sleep_adjust() reads
RK817_GAS_GAUGE_Q_PRES_H3 into a local charge_uah variable and either calls
rk817_bat_ocv_recalibrate() or writes the credited charge to
RK817_GAS_GAUGE_Q_INIT_H3 without updating charger->charge_now_uah.

If the system suspends during the 8-second window after a recalibration, or
after rk817_resume() before the queued charger->work executes (which
cancel_delayed_work_sync(&charger->work) then cancels), wouldn't
rk817_suspend() stamp the stale charger->charge_now_uah into
charger->slp_charge_uah and skew moved_uah and credit_uah on the next resume?

> +	charger->slp_cur_ua = abs(charger->cur_avg_ua);
> +	charger->slp_charging = charger->plugged_in &&
> +				(charger->charge_status == CC_OR_CV_CHRG ||
> +				 charger->charge_status == TRICKLE_CHRG);

[Severity: Medium]
Could taking abs(charger->cur_avg_ua) in rk817_suspend() cause
rk817_bat_sleep_adjust() to credit discharge current as charge?

In rk817_read_props(), charger->cur_avg_ua is a signed value where negative
readings indicate net battery discharge, which rk817_bat_get_prop() checks
before inspecting charger->charge_status:

drivers/power/supply/rk817_charger.c:rk817_bat_get_prop() {
    ...
	case POWER_SUPPLY_PROP_STATUS:
		if (charger->cur_avg_ua < 0) {
			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
			break;
		}
    ...
}

If the system suspends while plugged in with charge_status == CC_OR_CV_CHRG or
TRICKLE_CHRG while charger->cur_avg_ua < 0 (for instance, when pre-suspend
system load exceeds the input current limit, or right after plug-in before the
average current register slews positive), rk817_suspend() sets
charger->slp_charging = true and turns the negative current into a positive
charger->slp_cur_ua. On resume, wouldn't rk817_bat_sleep_adjust() then compute
a positive expected_uah from that discharge rate and credit phantom charge to
charger->soc, RK817_GAS_GAUGE_Q_INIT_H3, and NVRAM?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026174834.1652309-1-macroalpha82@gmail.com?part=3

Re: [PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Heiko Stübner]

Hi Chris,

Am Montag, 26. Oktober 2026, 18:48:33 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add a boolean attribute for the rk817-charger to control whether or not
> the system power is shutdown in battery off mode. The BSP kernel sets
> this value unless a device-tree attribute is present, and this attribute
> is to replicate that behavior.

Devicetree is supposed to describe the hardware, so your description needs
to describe the per-board property this represents.

Just because it's in a vendor-BSP is generally not enough justification :-) .

This is especially true as you wrote in the cover-letter that no known
board actually needs this right now.


Heiko

> Fixes: a592aa3a2a8c ("dt-bindings: mfd: Add Rockchip rk817 battery charger support")
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Suggested-by: Zetarancio <zetarancio@users.noreply.github.com>
> ---
>  Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> index 2cb6d176a84c..4cf987fe763b 100644
> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> @@ -148,6 +148,12 @@ properties:
>            Value in microamps of the sleep filter current for the charger.
>            Value is used by the driver to derive the sleep sample current.
>  
> +      rockchip,gate-function-disable:
> +        type: boolean
> +        description:
> +          Boolean value to prevent PMIC from turning off system voltage when
> +          battery voltage is shut down.
> +
>      required:
>        - monitored-battery
>        - rockchip,resistor-sense-micro-ohms
> @@ -381,6 +387,7 @@ examples:
>  
>              rk817_charger: charger {
>                  monitored-battery = <&battery>;
> +                rockchip,gate-function-disable;
>                  rockchip,resistor-sense-micro-ohms = <10000>;
>                  rockchip,sleep-enter-current-microamp = <300000>;
>                  rockchip,sleep-filter-current-microamp = <100000>;
> 

Re: [PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Heiko Stübner]

Hi Chris,

Am Montag, 26. Oktober 2026, 18:48:33 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add a boolean attribute for the rk817-charger to control whether or not
> the system power is shutdown in battery off mode. The BSP kernel sets
> this value unless a device-tree attribute is present, and this attribute
> is to replicate that behavior.

Devicetree is supposed to describe the hardware, so your description needs
to describe the per-board property this represents.

Just because it's in a vendor-BSP is generally not enough justification :-) .

This is especially true as you wrote in the cover-letter that no known
board actually needs this right now.


Heiko

> Fixes: a592aa3a2a8c ("dt-bindings: mfd: Add Rockchip rk817 battery charger support")
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Suggested-by: Zetarancio <zetarancio@users.noreply.github.com>
> ---
>  Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> index 2cb6d176a84c..4cf987fe763b 100644
> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> @@ -148,6 +148,12 @@ properties:
>            Value in microamps of the sleep filter current for the charger.
>            Value is used by the driver to derive the sleep sample current.
>  
> +      rockchip,gate-function-disable:
> +        type: boolean
> +        description:
> +          Boolean value to prevent PMIC from turning off system voltage when
> +          battery voltage is shut down.
> +
>      required:
>        - monitored-battery
>        - rockchip,resistor-sense-micro-ohms
> @@ -381,6 +387,7 @@ examples:
>  
>              rk817_charger: charger {
>                  monitored-battery = <&battery>;
> +                rockchip,gate-function-disable;
>                  rockchip,resistor-sense-micro-ohms = <10000>;
>                  rockchip,sleep-enter-current-microamp = <300000>;
>                  rockchip,sleep-filter-current-microamp = <100000>;
> 





_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Chris Morgan]

On Mon, Oct 26, 2026 at 08:23:40PM +0100, Heiko Stübner wrote:
> Hi Chris,
> 
> Am Montag, 26. Oktober 2026, 18:48:33 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add a boolean attribute for the rk817-charger to control whether or not
> > the system power is shutdown in battery off mode. The BSP kernel sets
> > this value unless a device-tree attribute is present, and this attribute
> > is to replicate that behavior.
> 
> Devicetree is supposed to describe the hardware, so your description needs
> to describe the per-board property this represents.
> 
> Just because it's in a vendor-BSP is generally not enough justification :-) .
> 
> This is especially true as you wrote in the cover-letter that no known
> board actually needs this right now.
> 
> 
> Heiko

The datasheet describes this as "Whether the system voltage can be
shutdown in Bat_off mode" and the register itself is described as
SYS_CAN_SD. Maybe call it "rockchip,can-shutdown-bat-off"? Or given the
fact that I'm not aware of any boards that would need it today, should
I even bother with adding the devicetree override? The only reason I
am adding it is in case this "fix" introduces a regression it will
allow someone to set this flag, just like the BSP does.

Thank you.

> 
> > Fixes: a592aa3a2a8c ("dt-bindings: mfd: Add Rockchip rk817 battery charger support")
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > Suggested-by: Zetarancio <zetarancio@users.noreply.github.com>
> > ---
> >  Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> > index 2cb6d176a84c..4cf987fe763b 100644
> > --- a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> > @@ -148,6 +148,12 @@ properties:
> >            Value in microamps of the sleep filter current for the charger.
> >            Value is used by the driver to derive the sleep sample current.
> >  
> > +      rockchip,gate-function-disable:
> > +        type: boolean
> > +        description:
> > +          Boolean value to prevent PMIC from turning off system voltage when
> > +          battery voltage is shut down.
> > +
> >      required:
> >        - monitored-battery
> >        - rockchip,resistor-sense-micro-ohms
> > @@ -381,6 +387,7 @@ examples:
> >  
> >              rk817_charger: charger {
> >                  monitored-battery = <&battery>;
> > +                rockchip,gate-function-disable;
> >                  rockchip,resistor-sense-micro-ohms = <10000>;
> >                  rockchip,sleep-enter-current-microamp = <300000>;
> >                  rockchip,sleep-filter-current-microamp = <100000>;
> > 
> 
> 
> 
> 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Chris Morgan]

On Mon, Oct 26, 2026 at 08:23:40PM +0100, Heiko Stübner wrote:
> Hi Chris,
> 
> Am Montag, 26. Oktober 2026, 18:48:33 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add a boolean attribute for the rk817-charger to control whether or not
> > the system power is shutdown in battery off mode. The BSP kernel sets
> > this value unless a device-tree attribute is present, and this attribute
> > is to replicate that behavior.
> 
> Devicetree is supposed to describe the hardware, so your description needs
> to describe the per-board property this represents.
> 
> Just because it's in a vendor-BSP is generally not enough justification :-) .
> 
> This is especially true as you wrote in the cover-letter that no known
> board actually needs this right now.
> 
> 
> Heiko

The datasheet describes this as "Whether the system voltage can be
shutdown in Bat_off mode" and the register itself is described as
SYS_CAN_SD. Maybe call it "rockchip,can-shutdown-bat-off"? Or given the
fact that I'm not aware of any boards that would need it today, should
I even bother with adding the devicetree override? The only reason I
am adding it is in case this "fix" introduces a regression it will
allow someone to set this flag, just like the BSP does.

Thank you.

> 
> > Fixes: a592aa3a2a8c ("dt-bindings: mfd: Add Rockchip rk817 battery charger support")
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > Suggested-by: Zetarancio <zetarancio@users.noreply.github.com>
> > ---
> >  Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> > index 2cb6d176a84c..4cf987fe763b 100644
> > --- a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> > @@ -148,6 +148,12 @@ properties:
> >            Value in microamps of the sleep filter current for the charger.
> >            Value is used by the driver to derive the sleep sample current.
> >  
> > +      rockchip,gate-function-disable:
> > +        type: boolean
> > +        description:
> > +          Boolean value to prevent PMIC from turning off system voltage when
> > +          battery voltage is shut down.
> > +
> >      required:
> >        - monitored-battery
> >        - rockchip,resistor-sense-micro-ohms
> > @@ -381,6 +387,7 @@ examples:
> >  
> >              rk817_charger: charger {
> >                  monitored-battery = <&battery>;
> > +                rockchip,gate-function-disable;
> >                  rockchip,resistor-sense-micro-ohms = <10000>;
> >                  rockchip,sleep-enter-current-microamp = <300000>;
> >                  rockchip,sleep-filter-current-microamp = <100000>;
> > 
> 
> 
> 
> 

Re: [PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Heiko Stübner]

Am Dienstag, 27. Oktober 2026, 16:53:10 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> On Mon, Oct 26, 2026 at 08:23:40PM +0100, Heiko Stübner wrote:
> > Hi Chris,
> > 
> > Am Montag, 26. Oktober 2026, 18:48:33 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Add a boolean attribute for the rk817-charger to control whether or not
> > > the system power is shutdown in battery off mode. The BSP kernel sets
> > > this value unless a device-tree attribute is present, and this attribute
> > > is to replicate that behavior.
> > 
> > Devicetree is supposed to describe the hardware, so your description needs
> > to describe the per-board property this represents.
> > 
> > Just because it's in a vendor-BSP is generally not enough justification :-) .
> > 
> > This is especially true as you wrote in the cover-letter that no known
> > board actually needs this right now.
> > 
> > 
> > Heiko
> 
> The datasheet describes this as "Whether the system voltage can be
> shutdown in Bat_off mode" and the register itself is described as
> SYS_CAN_SD. Maybe call it "rockchip,can-shutdown-bat-off"? Or given the
> fact that I'm not aware of any boards that would need it today, should
> I even bother with adding the devicetree override? The only reason I
> am adding it is in case this "fix" introduces a regression it will
> allow someone to set this flag, just like the BSP does.

First of all, I guess defining the terms better would be helpful, i.e.
I don't know what that "battery off" mode is plus the whole enviroment
this happens in.

Secondly, this new flag would cause a regression, because a board needing
the old way would need to add that flag. But when a board actually
requiring this turns up with an older devicetree without that new flag
this breaks backwards compatiblity.

So if we have a function flag for the pmic, it would need to be the other
way around.

As you said this is specific to battery powered devices, of which we don't
have that many - especially in-tree. Can you maybe look in the vendor-tree
if any board actually uses that?


Heiko

Re: [PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Heiko Stübner]

Am Dienstag, 27. Oktober 2026, 16:53:10 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> On Mon, Oct 26, 2026 at 08:23:40PM +0100, Heiko Stübner wrote:
> > Hi Chris,
> > 
> > Am Montag, 26. Oktober 2026, 18:48:33 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Add a boolean attribute for the rk817-charger to control whether or not
> > > the system power is shutdown in battery off mode. The BSP kernel sets
> > > this value unless a device-tree attribute is present, and this attribute
> > > is to replicate that behavior.
> > 
> > Devicetree is supposed to describe the hardware, so your description needs
> > to describe the per-board property this represents.
> > 
> > Just because it's in a vendor-BSP is generally not enough justification :-) .
> > 
> > This is especially true as you wrote in the cover-letter that no known
> > board actually needs this right now.
> > 
> > 
> > Heiko
> 
> The datasheet describes this as "Whether the system voltage can be
> shutdown in Bat_off mode" and the register itself is described as
> SYS_CAN_SD. Maybe call it "rockchip,can-shutdown-bat-off"? Or given the
> fact that I'm not aware of any boards that would need it today, should
> I even bother with adding the devicetree override? The only reason I
> am adding it is in case this "fix" introduces a regression it will
> allow someone to set this flag, just like the BSP does.

First of all, I guess defining the terms better would be helpful, i.e.
I don't know what that "battery off" mode is plus the whole enviroment
this happens in.

Secondly, this new flag would cause a regression, because a board needing
the old way would need to add that flag. But when a board actually
requiring this turns up with an older devicetree without that new flag
this breaks backwards compatiblity.

So if we have a function flag for the pmic, it would need to be the other
way around.

As you said this is specific to battery powered devices, of which we don't
have that many - especially in-tree. Can you maybe look in the vendor-tree
if any board actually uses that?


Heiko



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Krzysztof Kozlowski]

On Tue, Oct 27, 2026 at 10:53:10AM -0500, Chris Morgan wrote:
> On Mon, Oct 26, 2026 at 08:23:40PM +0100, Heiko Stübner wrote:
> > Hi Chris,
> > 
> > Am Montag, 26. Oktober 2026, 18:48:33 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Add a boolean attribute for the rk817-charger to control whether or not
> > > the system power is shutdown in battery off mode. The BSP kernel sets
> > > this value unless a device-tree attribute is present, and this attribute
> > > is to replicate that behavior.
> > 
> > Devicetree is supposed to describe the hardware, so your description needs
> > to describe the per-board property this represents.
> > 
> > Just because it's in a vendor-BSP is generally not enough justification :-) .
> > 
> > This is especially true as you wrote in the cover-letter that no known
> > board actually needs this right now.
> > 
> > 
> > Heiko
> 
> The datasheet describes this as "Whether the system voltage can be
> shutdown in Bat_off mode" and the register itself is described as
> SYS_CAN_SD. Maybe call it "rockchip,can-shutdown-bat-off"? Or given the
> fact that I'm not aware of any boards that would need it today, should

Why would you like to add dead code?

Anyway, describe what problem is with the hardware that such property is
needed.

Best regards,
Krzysztof

Re: [PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Krzysztof Kozlowski]

On Tue, Oct 27, 2026 at 10:53:10AM -0500, Chris Morgan wrote:
> On Mon, Oct 26, 2026 at 08:23:40PM +0100, Heiko Stübner wrote:
> > Hi Chris,
> > 
> > Am Montag, 26. Oktober 2026, 18:48:33 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Add a boolean attribute for the rk817-charger to control whether or not
> > > the system power is shutdown in battery off mode. The BSP kernel sets
> > > this value unless a device-tree attribute is present, and this attribute
> > > is to replicate that behavior.
> > 
> > Devicetree is supposed to describe the hardware, so your description needs
> > to describe the per-board property this represents.
> > 
> > Just because it's in a vendor-BSP is generally not enough justification :-) .
> > 
> > This is especially true as you wrote in the cover-letter that no known
> > board actually needs this right now.
> > 
> > 
> > Heiko
> 
> The datasheet describes this as "Whether the system voltage can be
> shutdown in Bat_off mode" and the register itself is described as
> SYS_CAN_SD. Maybe call it "rockchip,can-shutdown-bat-off"? Or given the
> fact that I'm not aware of any boards that would need it today, should

Why would you like to add dead code?

Anyway, describe what problem is with the hardware that such property is
needed.

Best regards,
Krzysztof


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Chris Morgan]

On Tue, Oct 27, 2026 at 11:40:46PM +0100, Heiko Stübner wrote:
> Am Dienstag, 27. Oktober 2026, 16:53:10 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> > On Mon, Oct 26, 2026 at 08:23:40PM +0100, Heiko Stübner wrote:
> > > Hi Chris,
> > > 
> > > Am Montag, 26. Oktober 2026, 18:48:33 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > 
> > > > Add a boolean attribute for the rk817-charger to control whether or not
> > > > the system power is shutdown in battery off mode. The BSP kernel sets
> > > > this value unless a device-tree attribute is present, and this attribute
> > > > is to replicate that behavior.
> > > 
> > > Devicetree is supposed to describe the hardware, so your description needs
> > > to describe the per-board property this represents.
> > > 
> > > Just because it's in a vendor-BSP is generally not enough justification :-) .
> > > 
> > > This is especially true as you wrote in the cover-letter that no known
> > > board actually needs this right now.
> > > 
> > > 
> > > Heiko
> > 
> > The datasheet describes this as "Whether the system voltage can be
> > shutdown in Bat_off mode" and the register itself is described as
> > SYS_CAN_SD. Maybe call it "rockchip,can-shutdown-bat-off"? Or given the
> > fact that I'm not aware of any boards that would need it today, should
> > I even bother with adding the devicetree override? The only reason I
> > am adding it is in case this "fix" introduces a regression it will
> > allow someone to set this flag, just like the BSP does.
> 
> First of all, I guess defining the terms better would be helpful, i.e.
> I don't know what that "battery off" mode is plus the whole enviroment
> this happens in.
> 
> Secondly, this new flag would cause a regression, because a board needing
> the old way would need to add that flag. But when a board actually
> requiring this turns up with an older devicetree without that new flag
> this breaks backwards compatiblity.
> 
> So if we have a function flag for the pmic, it would need to be the other
> way around.
> 
> As you said this is specific to battery powered devices, of which we don't
> have that many - especially in-tree. Can you maybe look in the vendor-tree
> if any board actually uses that?
> 
> 
> Heiko
> 
> 

In the next revision I'll just enable it, and put a note inline
stating that BSP offered a way to disable this behavior but I have
never observed a device doing so. The documentation around what this
bit does isn't exactly clear but from what I gather it sets the output
voltage of the battery to on even when the PMIC is off. Otherwise, when
the PMIC is off the battery output voltage is off. I see that at least
a few devices from Trim UI experience a power drain while off if this
bit is not cleared. Not sure why keeping the battery *on* fixes a power
drain, but I haven't noticed any other devices experiencing a
regression with this bit cleared, a few devices work better with this
bit cleared, and the BSP did it this way.

So next revision I'll just drop the devicetree pieces and always clear
the bit.

Thank you,
Chris

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Chris Morgan]

On Tue, Oct 27, 2026 at 11:40:46PM +0100, Heiko Stübner wrote:
> Am Dienstag, 27. Oktober 2026, 16:53:10 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> > On Mon, Oct 26, 2026 at 08:23:40PM +0100, Heiko Stübner wrote:
> > > Hi Chris,
> > > 
> > > Am Montag, 26. Oktober 2026, 18:48:33 Mitteleuropäische Normalzeit schrieb Chris Morgan:
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > 
> > > > Add a boolean attribute for the rk817-charger to control whether or not
> > > > the system power is shutdown in battery off mode. The BSP kernel sets
> > > > this value unless a device-tree attribute is present, and this attribute
> > > > is to replicate that behavior.
> > > 
> > > Devicetree is supposed to describe the hardware, so your description needs
> > > to describe the per-board property this represents.
> > > 
> > > Just because it's in a vendor-BSP is generally not enough justification :-) .
> > > 
> > > This is especially true as you wrote in the cover-letter that no known
> > > board actually needs this right now.
> > > 
> > > 
> > > Heiko
> > 
> > The datasheet describes this as "Whether the system voltage can be
> > shutdown in Bat_off mode" and the register itself is described as
> > SYS_CAN_SD. Maybe call it "rockchip,can-shutdown-bat-off"? Or given the
> > fact that I'm not aware of any boards that would need it today, should
> > I even bother with adding the devicetree override? The only reason I
> > am adding it is in case this "fix" introduces a regression it will
> > allow someone to set this flag, just like the BSP does.
> 
> First of all, I guess defining the terms better would be helpful, i.e.
> I don't know what that "battery off" mode is plus the whole enviroment
> this happens in.
> 
> Secondly, this new flag would cause a regression, because a board needing
> the old way would need to add that flag. But when a board actually
> requiring this turns up with an older devicetree without that new flag
> this breaks backwards compatiblity.
> 
> So if we have a function flag for the pmic, it would need to be the other
> way around.
> 
> As you said this is specific to battery powered devices, of which we don't
> have that many - especially in-tree. Can you maybe look in the vendor-tree
> if any board actually uses that?
> 
> 
> Heiko
> 
> 

In the next revision I'll just enable it, and put a note inline
stating that BSP offered a way to disable this behavior but I have
never observed a device doing so. The documentation around what this
bit does isn't exactly clear but from what I gather it sets the output
voltage of the battery to on even when the PMIC is off. Otherwise, when
the PMIC is off the battery output voltage is off. I see that at least
a few devices from Trim UI experience a power drain while off if this
bit is not cleared. Not sure why keeping the battery *on* fixes a power
drain, but I haven't noticed any other devices experiencing a
regression with this bit cleared, a few devices work better with this
bit cleared, and the BSP did it this way.

So next revision I'll just drop the devicetree pieces and always clear
the bit.

Thank you,
Chris

Re: [PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Krzysztof Kozlowski]

On Mon, Oct 26, 2026 at 12:48:33PM -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add a boolean attribute for the rk817-charger to control whether or not
> the system power is shutdown in battery off mode. The BSP kernel sets
> this value unless a device-tree attribute is present, and this attribute
> is to replicate that behavior.
> 
> Fixes: a592aa3a2a8c ("dt-bindings: mfd: Add Rockchip rk817 battery charger support")
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Suggested-by: Zetarancio <zetarancio@users.noreply.github.com>

We do not need to credit non-real identities (like nicknames) without
even pieces of known identity. Just drop the tag.

Also, your SoB is the last one.

Best regards,
Krzysztof

Re: [PATCH 4/5] dt-bindings: power: supply: rk817-charger: Add rockchip,gate-function-disable

[Krzysztof Kozlowski]

On Mon, Oct 26, 2026 at 12:48:33PM -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add a boolean attribute for the rk817-charger to control whether or not
> the system power is shutdown in battery off mode. The BSP kernel sets
> this value unless a device-tree attribute is present, and this attribute
> is to replicate that behavior.
> 
> Fixes: a592aa3a2a8c ("dt-bindings: mfd: Add Rockchip rk817 battery charger support")
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Suggested-by: Zetarancio <zetarancio@users.noreply.github.com>

We do not need to credit non-real identities (like nicknames) without
even pieces of known identity. Just drop the tag.

Also, your SoB is the last one.

Best regards,
Krzysztof


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 3/5] power: supply: rk817-charger: Correct the gauge across system sleep

[Krzysztof Kozlowski]

On Mon, Oct 26, 2026 at 12:48:32PM -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> The coulomb counter is erratic while the system sleeps. On an RG353M
> charging at 1.4A, one 15 minute sleep registered 69% of the metered
> charge and another 139%, and a charge that finished during sleep still
> showed the pre-sleep percentage at wake. The signals that could correct
> it are gone by the time the driver runs again: RELAX_STS clears as soon
> as the wakeup load exceeds the relax threshold (300mA here, less than
> an idle UI draws), and CHARGE_FINISH disappears while the charger
> re-detects its input. The BSP driver corrects at resume from sleep time
> times current and from the relax voltage. Do the same from evidence
> that persists:
> 
> - At suspend, stamp boottime, charge, current and charging state, and
>   clear the relax-updated flags.
> 
> - At resume, recompute the soc from the counter, capped at full. A
>   relax pair with its flags set was latched during this sleep:
>   recalibrate from it without demanding a live RELAX_STS. Otherwise,
>   if the system slept charging, credit elapsed * current where the
>   counter fell short, saturating at full, which also covers a charge
>   that finished during sleep.
> 
> The constant-current estimate reads high across a CV taper; the cap
> bounds it and the next rested pair corrects the remainder.
> 
> Log the counter at suspend and resume, the battery voltage and the
> relax pair, so the path taken can be read from the log.
> 
> Suggested-by: Jacob Cook <jacobmcook1984@gmail.com>
> Co-developed-by: Jacob Cook <jacobmcook1984@gmail.com>
> Signed-off-by: Jacob Cook <jacobmcook1984@gmail.com>

One cannot suggest own work. How would that work:
Krzysztof 1: Hi Krzysztof, my name is Krzysztof, would you do foo-bar?
Krzysztof 2: Yes, Krzysztof, of course, excellent idea, let me credit
you in Suggested-by.

> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Assisted-by: Claude:claude-5-fable

Drop, not correct format for some time, but regardless checkpatch should
complain.

Please run scripts/checkpatch.pl on the patches and fix reported
warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
patches and (probably) fix more warnings. Some warnings can be ignored,
especially from --strict run, but the code here looks like it needs a
fix. Feel free to get in touch if the warning is not clear.

Best regards,
Krzysztof

Re: [PATCH 3/5] power: supply: rk817-charger: Correct the gauge across system sleep

[Krzysztof Kozlowski]

On Mon, Oct 26, 2026 at 12:48:32PM -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> The coulomb counter is erratic while the system sleeps. On an RG353M
> charging at 1.4A, one 15 minute sleep registered 69% of the metered
> charge and another 139%, and a charge that finished during sleep still
> showed the pre-sleep percentage at wake. The signals that could correct
> it are gone by the time the driver runs again: RELAX_STS clears as soon
> as the wakeup load exceeds the relax threshold (300mA here, less than
> an idle UI draws), and CHARGE_FINISH disappears while the charger
> re-detects its input. The BSP driver corrects at resume from sleep time
> times current and from the relax voltage. Do the same from evidence
> that persists:
> 
> - At suspend, stamp boottime, charge, current and charging state, and
>   clear the relax-updated flags.
> 
> - At resume, recompute the soc from the counter, capped at full. A
>   relax pair with its flags set was latched during this sleep:
>   recalibrate from it without demanding a live RELAX_STS. Otherwise,
>   if the system slept charging, credit elapsed * current where the
>   counter fell short, saturating at full, which also covers a charge
>   that finished during sleep.
> 
> The constant-current estimate reads high across a CV taper; the cap
> bounds it and the next rested pair corrects the remainder.
> 
> Log the counter at suspend and resume, the battery voltage and the
> relax pair, so the path taken can be read from the log.
> 
> Suggested-by: Jacob Cook <jacobmcook1984@gmail.com>
> Co-developed-by: Jacob Cook <jacobmcook1984@gmail.com>
> Signed-off-by: Jacob Cook <jacobmcook1984@gmail.com>

One cannot suggest own work. How would that work:
Krzysztof 1: Hi Krzysztof, my name is Krzysztof, would you do foo-bar?
Krzysztof 2: Yes, Krzysztof, of course, excellent idea, let me credit
you in Suggested-by.

> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Assisted-by: Claude:claude-5-fable

Drop, not correct format for some time, but regardless checkpatch should
complain.

Please run scripts/checkpatch.pl on the patches and fix reported
warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
patches and (probably) fix more warnings. Some warnings can be ignored,
especially from --strict run, but the code here looks like it needs a
fix. Feel free to get in touch if the warning is not clear.

Best regards,
Krzysztof


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip