* [PATCH 1/2] power: supply: qcom_battmgr: clamp charge control thresholds
2025-10-12 23:32 [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling Val Packett
@ 2025-10-12 23:32 ` Val Packett
2025-10-13 9:43 ` Konrad Dybcio
2025-10-12 23:32 ` [PATCH 2/2] power: supply: qcom_battmgr: support disabling charge control Val Packett
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Val Packett @ 2025-10-12 23:32 UTC (permalink / raw)
To: Sebastian Reichel, Neil Armstrong, Fenglin Wu
Cc: Val Packett, Sebastian Reichel, linux-arm-msm, linux-pm,
linux-kernel
The sysfs API documentation says that drivers "round written values to
the nearest supported value" for charge_control_end_threshold.
Let's do this for both thresholds, as userspace (e.g. upower) generally
does not expect these writes to fail at all.
Fixes: cc3e883a0625 ("power: supply: qcom_battmgr: Add charge control support")
Signed-off-by: Val Packett <val@packett.cool>
---
drivers/power/supply/qcom_battmgr.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
index 3c2837ef3461..c8028606bba0 100644
--- a/drivers/power/supply/qcom_battmgr.c
+++ b/drivers/power/supply/qcom_battmgr.c
@@ -678,12 +678,7 @@ static int qcom_battmgr_set_charge_start_threshold(struct qcom_battmgr *battmgr,
u32 target_soc, delta_soc;
int ret;
- if (start_soc < CHARGE_CTRL_START_THR_MIN ||
- start_soc > CHARGE_CTRL_START_THR_MAX) {
- dev_err(battmgr->dev, "charge control start threshold exceed range: [%u - %u]\n",
- CHARGE_CTRL_START_THR_MIN, CHARGE_CTRL_START_THR_MAX);
- return -EINVAL;
- }
+ start_soc = clamp(start_soc, CHARGE_CTRL_START_THR_MIN, CHARGE_CTRL_START_THR_MAX);
/*
* If the new start threshold is larger than the old end threshold,
@@ -716,12 +711,7 @@ static int qcom_battmgr_set_charge_end_threshold(struct qcom_battmgr *battmgr, i
u32 delta_soc = CHARGE_CTRL_DELTA_SOC;
int ret;
- if (end_soc < CHARGE_CTRL_END_THR_MIN ||
- end_soc > CHARGE_CTRL_END_THR_MAX) {
- dev_err(battmgr->dev, "charge control end threshold exceed range: [%u - %u]\n",
- CHARGE_CTRL_END_THR_MIN, CHARGE_CTRL_END_THR_MAX);
- return -EINVAL;
- }
+ end_soc = clamp(end_soc, CHARGE_CTRL_END_THR_MIN, CHARGE_CTRL_END_THR_MAX);
if (battmgr->info.charge_ctrl_start && end_soc > battmgr->info.charge_ctrl_start)
delta_soc = end_soc - battmgr->info.charge_ctrl_start;
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] power: supply: qcom_battmgr: clamp charge control thresholds
2025-10-12 23:32 ` [PATCH 1/2] power: supply: qcom_battmgr: clamp charge control thresholds Val Packett
@ 2025-10-13 9:43 ` Konrad Dybcio
0 siblings, 0 replies; 12+ messages in thread
From: Konrad Dybcio @ 2025-10-13 9:43 UTC (permalink / raw)
To: Val Packett, Sebastian Reichel, Neil Armstrong, Fenglin Wu
Cc: Sebastian Reichel, linux-arm-msm, linux-pm, linux-kernel
On 10/13/25 1:32 AM, Val Packett wrote:
> The sysfs API documentation says that drivers "round written values to
> the nearest supported value" for charge_control_end_threshold.
>
> Let's do this for both thresholds, as userspace (e.g. upower) generally
> does not expect these writes to fail at all.
The documentation says so only for the upper bound. You should
probably submit a patch to amend the lower one as well
Konrad
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] power: supply: qcom_battmgr: support disabling charge control
2025-10-12 23:32 [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling Val Packett
2025-10-12 23:32 ` [PATCH 1/2] power: supply: qcom_battmgr: clamp charge control thresholds Val Packett
@ 2025-10-12 23:32 ` Val Packett
2025-11-17 5:22 ` Fenglin Wu
2025-11-03 0:48 ` [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling Sebastian Reichel
2025-11-17 5:12 ` Fenglin Wu
3 siblings, 1 reply; 12+ messages in thread
From: Val Packett @ 2025-10-12 23:32 UTC (permalink / raw)
To: Sebastian Reichel, Fenglin Wu, Neil Armstrong
Cc: Val Packett, Sebastian Reichel, linux-arm-msm, linux-pm,
linux-kernel
Existing userspace (in particular, upower) disables charge control by
setting the start threshold to 0 and the stop threshold to 100.
Handle that by actually setting the enable bit to 0 when a start
threshold of 0 was requested.
Fixes: cc3e883a0625 ("power: supply: qcom_battmgr: Add charge control support")
Signed-off-by: Val Packett <val@packett.cool>
---
drivers/power/supply/qcom_battmgr.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
index c8028606bba0..e6f01e0122e1 100644
--- a/drivers/power/supply/qcom_battmgr.c
+++ b/drivers/power/supply/qcom_battmgr.c
@@ -257,6 +257,7 @@ struct qcom_battmgr_info {
unsigned int capacity_warning;
unsigned int cycle_count;
unsigned int charge_count;
+ bool charge_ctrl_enable;
unsigned int charge_ctrl_start;
unsigned int charge_ctrl_end;
char model_number[BATTMGR_STRING_LEN];
@@ -659,13 +660,13 @@ static int qcom_battmgr_bat_get_property(struct power_supply *psy,
}
static int qcom_battmgr_set_charge_control(struct qcom_battmgr *battmgr,
- u32 target_soc, u32 delta_soc)
+ bool enable, u32 target_soc, u32 delta_soc)
{
struct qcom_battmgr_charge_ctrl_request request = {
.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_BATTMGR),
.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
.hdr.opcode = cpu_to_le32(BATTMGR_CHG_CTRL_LIMIT_EN),
- .enable = cpu_to_le32(1),
+ .enable = cpu_to_le32(enable),
.target_soc = cpu_to_le32(target_soc),
.delta_soc = cpu_to_le32(delta_soc),
};
@@ -677,6 +678,7 @@ static int qcom_battmgr_set_charge_start_threshold(struct qcom_battmgr *battmgr,
{
u32 target_soc, delta_soc;
int ret;
+ bool enable = start_soc != 0;
start_soc = clamp(start_soc, CHARGE_CTRL_START_THR_MIN, CHARGE_CTRL_START_THR_MAX);
@@ -696,9 +698,10 @@ static int qcom_battmgr_set_charge_start_threshold(struct qcom_battmgr *battmgr,
}
mutex_lock(&battmgr->lock);
- ret = qcom_battmgr_set_charge_control(battmgr, target_soc, delta_soc);
+ ret = qcom_battmgr_set_charge_control(battmgr, enable, target_soc, delta_soc);
mutex_unlock(&battmgr->lock);
if (!ret) {
+ battmgr->info.charge_ctrl_enable = enable;
battmgr->info.charge_ctrl_start = start_soc;
battmgr->info.charge_ctrl_end = target_soc;
}
@@ -710,6 +713,7 @@ static int qcom_battmgr_set_charge_end_threshold(struct qcom_battmgr *battmgr, i
{
u32 delta_soc = CHARGE_CTRL_DELTA_SOC;
int ret;
+ bool enable = battmgr->info.charge_ctrl_enable;
end_soc = clamp(end_soc, CHARGE_CTRL_END_THR_MIN, CHARGE_CTRL_END_THR_MAX);
@@ -717,7 +721,7 @@ static int qcom_battmgr_set_charge_end_threshold(struct qcom_battmgr *battmgr, i
delta_soc = end_soc - battmgr->info.charge_ctrl_start;
mutex_lock(&battmgr->lock);
- ret = qcom_battmgr_set_charge_control(battmgr, end_soc, delta_soc);
+ ret = qcom_battmgr_set_charge_control(battmgr, enable, end_soc, delta_soc);
mutex_unlock(&battmgr->lock);
if (!ret) {
battmgr->info.charge_ctrl_start = end_soc - delta_soc;
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] power: supply: qcom_battmgr: support disabling charge control
2025-10-12 23:32 ` [PATCH 2/2] power: supply: qcom_battmgr: support disabling charge control Val Packett
@ 2025-11-17 5:22 ` Fenglin Wu
0 siblings, 0 replies; 12+ messages in thread
From: Fenglin Wu @ 2025-11-17 5:22 UTC (permalink / raw)
To: Val Packett, Sebastian Reichel, Neil Armstrong
Cc: Sebastian Reichel, linux-arm-msm, linux-pm, linux-kernel
On 10/13/2025 7:32 AM, Val Packett wrote:
> Existing userspace (in particular, upower) disables charge control by
> setting the start threshold to 0 and the stop threshold to 100.
>
> Handle that by actually setting the enable bit to 0 when a start
> threshold of 0 was requested.
>
> Fixes: cc3e883a0625 ("power: supply: qcom_battmgr: Add charge control support")
> Signed-off-by: Val Packett <val@packett.cool>
> ---
> drivers/power/supply/qcom_battmgr.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
> index c8028606bba0..e6f01e0122e1 100644
> --- a/drivers/power/supply/qcom_battmgr.c
> +++ b/drivers/power/supply/qcom_battmgr.c
> @@ -257,6 +257,7 @@ struct qcom_battmgr_info {
> unsigned int capacity_warning;
> unsigned int cycle_count;
> unsigned int charge_count;
> + bool charge_ctrl_enable;
> unsigned int charge_ctrl_start;
> unsigned int charge_ctrl_end;
> char model_number[BATTMGR_STRING_LEN];
> @@ -659,13 +660,13 @@ static int qcom_battmgr_bat_get_property(struct power_supply *psy,
> }
>
> static int qcom_battmgr_set_charge_control(struct qcom_battmgr *battmgr,
> - u32 target_soc, u32 delta_soc)
> + bool enable, u32 target_soc, u32 delta_soc)
> {
> struct qcom_battmgr_charge_ctrl_request request = {
> .hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_BATTMGR),
> .hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
> .hdr.opcode = cpu_to_le32(BATTMGR_CHG_CTRL_LIMIT_EN),
> - .enable = cpu_to_le32(1),
> + .enable = cpu_to_le32(enable),
> .target_soc = cpu_to_le32(target_soc),
> .delta_soc = cpu_to_le32(delta_soc),
> };
> @@ -677,6 +678,7 @@ static int qcom_battmgr_set_charge_start_threshold(struct qcom_battmgr *battmgr,
> {
> u32 target_soc, delta_soc;
> int ret;
> + bool enable = start_soc != 0;
>
> start_soc = clamp(start_soc, CHARGE_CTRL_START_THR_MIN, CHARGE_CTRL_START_THR_MAX);
>
> @@ -696,9 +698,10 @@ static int qcom_battmgr_set_charge_start_threshold(struct qcom_battmgr *battmgr,
> }
>
> mutex_lock(&battmgr->lock);
> - ret = qcom_battmgr_set_charge_control(battmgr, target_soc, delta_soc);
> + ret = qcom_battmgr_set_charge_control(battmgr, enable, target_soc, delta_soc);
> mutex_unlock(&battmgr->lock);
> if (!ret) {
> + battmgr->info.charge_ctrl_enable = enable;
> battmgr->info.charge_ctrl_start = start_soc;
> battmgr->info.charge_ctrl_end = target_soc;
> }
> @@ -710,6 +713,7 @@ static int qcom_battmgr_set_charge_end_threshold(struct qcom_battmgr *battmgr, i
> {
> u32 delta_soc = CHARGE_CTRL_DELTA_SOC;
> int ret;
> + bool enable = battmgr->info.charge_ctrl_enable;
Can you initialize "battmgr->info.charge_ctrl_enable" in
"qcom_battmgr_charge_control_thresholds_init()" based on the value
reading from the nvmem cell? Otherwise, it would have a false value by
default and a single write to the end threshold would result disabling
the charging control instead.
>
> end_soc = clamp(end_soc, CHARGE_CTRL_END_THR_MIN, CHARGE_CTRL_END_THR_MAX);
>
> @@ -717,7 +721,7 @@ static int qcom_battmgr_set_charge_end_threshold(struct qcom_battmgr *battmgr, i
> delta_soc = end_soc - battmgr->info.charge_ctrl_start;
>
> mutex_lock(&battmgr->lock);
> - ret = qcom_battmgr_set_charge_control(battmgr, end_soc, delta_soc);
> + ret = qcom_battmgr_set_charge_control(battmgr, enable, end_soc, delta_soc);
> mutex_unlock(&battmgr->lock);
> if (!ret) {
> battmgr->info.charge_ctrl_start = end_soc - delta_soc;
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling
2025-10-12 23:32 [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling Val Packett
2025-10-12 23:32 ` [PATCH 1/2] power: supply: qcom_battmgr: clamp charge control thresholds Val Packett
2025-10-12 23:32 ` [PATCH 2/2] power: supply: qcom_battmgr: support disabling charge control Val Packett
@ 2025-11-03 0:48 ` Sebastian Reichel
2025-11-03 3:46 ` Val Packett
2025-11-17 5:12 ` Fenglin Wu
3 siblings, 1 reply; 12+ messages in thread
From: Sebastian Reichel @ 2025-11-03 0:48 UTC (permalink / raw)
To: Sebastian Reichel, Fenglin Wu, Neil Armstrong, linux-arm-msm,
linux-pm, linux-kernel, Val Packett
On Sun, 12 Oct 2025 20:32:17 -0300, Val Packett wrote:
> Currently, upowerd is unable to turn off the battery preservation mode[1]
> on Qualcomm laptops, because it does that by setting the start threshold to
> zero and the driver returns an error:
>
> pmic_glink.power-supply.0: charge control start threshold exceed range: [50 - 95]
>
> Kernel documentation says the end threshold must be clamped[2] but does
> not say anything about the start threshold.
>
> [...]
Applied, thanks!
[1/2] power: supply: qcom_battmgr: clamp charge control thresholds
commit: 8809980fdc8a86070667032fa4005ee83f1c62f3
[2/2] power: supply: qcom_battmgr: support disabling charge control
commit: 446fcf494691da4e685923e5fad02b163955fc0e
Best regards,
--
Sebastian Reichel <sebastian.reichel@collabora.com>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling
2025-11-03 0:48 ` [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling Sebastian Reichel
@ 2025-11-03 3:46 ` Val Packett
2025-11-03 14:41 ` Sebastian Reichel
0 siblings, 1 reply; 12+ messages in thread
From: Val Packett @ 2025-11-03 3:46 UTC (permalink / raw)
To: Sebastian Reichel, Sebastian Reichel, Fenglin Wu, Neil Armstrong,
linux-arm-msm, linux-pm, linux-kernel
On 11/2/25 9:48 PM, Sebastian Reichel wrote:
> On Sun, 12 Oct 2025 20:32:17 -0300, Val Packett wrote:
>> Currently, upowerd is unable to turn off the battery preservation mode[1]
>> on Qualcomm laptops, because it does that by setting the start threshold to
>> zero and the driver returns an error:
>>
>> pmic_glink.power-supply.0: charge control start threshold exceed range: [50 - 95]
>>
>> Kernel documentation says the end threshold must be clamped[2] but does
>> not say anything about the start threshold.
>>
>> [...]
> Applied, thanks!
>
> [1/2] power: supply: qcom_battmgr: clamp charge control thresholds
> commit: 8809980fdc8a86070667032fa4005ee83f1c62f3
> [2/2] power: supply: qcom_battmgr: support disabling charge control
> commit: 446fcf494691da4e685923e5fad02b163955fc0e
Woahh.. please revert the second one.
I'm sorry, I thought this was discussed here but apparently it was only
on IRC and I must've assumed that the patches weren't going anywhere
because of the lack of R-b..
The disable bit was acting rather strange after all, we'd need more work
to figure out if that's even possible. Let's leave it at the clamp only.
~val
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling
2025-11-03 3:46 ` Val Packett
@ 2025-11-03 14:41 ` Sebastian Reichel
0 siblings, 0 replies; 12+ messages in thread
From: Sebastian Reichel @ 2025-11-03 14:41 UTC (permalink / raw)
To: Val Packett
Cc: Fenglin Wu, Neil Armstrong, linux-arm-msm, linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]
Hi,
On Mon, Nov 03, 2025 at 12:46:13AM -0300, Val Packett wrote:
> On 11/2/25 9:48 PM, Sebastian Reichel wrote:
>
> > On Sun, 12 Oct 2025 20:32:17 -0300, Val Packett wrote:
> > > Currently, upowerd is unable to turn off the battery preservation mode[1]
> > > on Qualcomm laptops, because it does that by setting the start threshold to
> > > zero and the driver returns an error:
> > >
> > > pmic_glink.power-supply.0: charge control start threshold exceed range: [50 - 95]
> > >
> > > Kernel documentation says the end threshold must be clamped[2] but does
> > > not say anything about the start threshold.
> > >
> > > [...]
> > Applied, thanks!
> >
> > [1/2] power: supply: qcom_battmgr: clamp charge control thresholds
> > commit: 8809980fdc8a86070667032fa4005ee83f1c62f3
> > [2/2] power: supply: qcom_battmgr: support disabling charge control
> > commit: 446fcf494691da4e685923e5fad02b163955fc0e
>
>
> Woahh.. please revert the second one.
>
> I'm sorry, I thought this was discussed here but apparently it was only on
> IRC and I must've assumed that the patches weren't going anywhere because of
> the lack of R-b..
>
> The disable bit was acting rather strange after all, we'd need more work to
> figure out if that's even possible. Let's leave it at the clamp only.
DONE.
Greetings,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling
2025-10-12 23:32 [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling Val Packett
` (2 preceding siblings ...)
2025-11-03 0:48 ` [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling Sebastian Reichel
@ 2025-11-17 5:12 ` Fenglin Wu
2025-11-17 12:45 ` Konrad Dybcio
3 siblings, 1 reply; 12+ messages in thread
From: Fenglin Wu @ 2025-11-17 5:12 UTC (permalink / raw)
To: Val Packett, Sebastian Reichel, Neil Armstrong, linux-arm-msm,
linux-pm, linux-kernel
On 10/13/2025 7:32 AM, Val Packett wrote:
> Currently, upowerd is unable to turn off the battery preservation mode[1]
> on Qualcomm laptops, because it does that by setting the start threshold to
> zero and the driver returns an error:
>
> pmic_glink.power-supply.0: charge control start threshold exceed range: [50 - 95]
>
> Kernel documentation says the end threshold must be clamped[2] but does
> not say anything about the start threshold.
>
> In this proposal I've special-cased start==0 to actually disable the
> functionality via the enable bit, and otherwise made both start and
> end thresholds be clamped to the acceptable range. Hopefully that's
> fine?
It is fine to clamping the threshold to the acceptable range. Thank you
for making the changes.
> Or should the [1 - 49] range for start actually be rejected?
The minimum charging start threshold was set to 50 to improve user
experience. If the threshold is too low and the system keeps drawing
power from the battery frequently due to a large system load and a weak
charger, the laptop will only begin charging when the battery level
falls below that threshold. If the user disconnects the charger at that
time, then the device would be only having a battery below 50%. Setting
the threshold at 50 ensures the battery always stays above 50%.
> [1]: https://gitlab.freedesktop.org/upower/upower/-/issues/327
> [2]: https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-class-power
>
> Thanks,
> ~val
>
> Val Packett (2):
> power: supply: qcom_battmgr: clamp charge control thresholds
> power: supply: qcom_battmgr: support disabling charge control
>
> drivers/power/supply/qcom_battmgr.c | 26 ++++++++++----------------
> 1 file changed, 10 insertions(+), 16 deletions(-)
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling
2025-11-17 5:12 ` Fenglin Wu
@ 2025-11-17 12:45 ` Konrad Dybcio
2025-11-18 2:29 ` Fenglin Wu
0 siblings, 1 reply; 12+ messages in thread
From: Konrad Dybcio @ 2025-11-17 12:45 UTC (permalink / raw)
To: Fenglin Wu, Val Packett, Sebastian Reichel, Neil Armstrong,
linux-arm-msm, linux-pm, linux-kernel
On 11/17/25 6:12 AM, Fenglin Wu wrote:
>
> On 10/13/2025 7:32 AM, Val Packett wrote:
>> Currently, upowerd is unable to turn off the battery preservation mode[1]
>> on Qualcomm laptops, because it does that by setting the start threshold to
>> zero and the driver returns an error:
>>
>> pmic_glink.power-supply.0: charge control start threshold exceed range: [50 - 95]
>>
>> Kernel documentation says the end threshold must be clamped[2] but does
>> not say anything about the start threshold.
>>
>> In this proposal I've special-cased start==0 to actually disable the
>> functionality via the enable bit, and otherwise made both start and
>> end thresholds be clamped to the acceptable range. Hopefully that's
>> fine?
> It is fine to clamping the threshold to the acceptable range. Thank you for making the changes.
>> Or should the [1 - 49] range for start actually be rejected?
> The minimum charging start threshold was set to 50 to improve user experience. If the threshold is too low and the system keeps drawing power from the battery frequently due to a large system load and a weak charger, the laptop will only begin charging when the battery level falls below that threshold. If the user disconnects the charger at that time, then the device would be only having a battery below 50%. Setting the threshold at 50 ensures the battery always stays above 50%.
So can we set it lower?
Such decisions are best deferred to userspace and/or the user, which can
limit what the kernel exposes as necessary/deemed useful
Konrad
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling
2025-11-17 12:45 ` Konrad Dybcio
@ 2025-11-18 2:29 ` Fenglin Wu
2025-11-18 10:13 ` Konrad Dybcio
0 siblings, 1 reply; 12+ messages in thread
From: Fenglin Wu @ 2025-11-18 2:29 UTC (permalink / raw)
To: Konrad Dybcio, Val Packett, Sebastian Reichel, Neil Armstrong,
linux-arm-msm, linux-pm, linux-kernel
On 11/17/2025 8:45 PM, Konrad Dybcio wrote:
> On 11/17/25 6:12 AM, Fenglin Wu wrote:
>> On 10/13/2025 7:32 AM, Val Packett wrote:
>>> Currently, upowerd is unable to turn off the battery preservation mode[1]
>>> on Qualcomm laptops, because it does that by setting the start threshold to
>>> zero and the driver returns an error:
>>>
>>> pmic_glink.power-supply.0: charge control start threshold exceed range: [50 - 95]
>>>
>>> Kernel documentation says the end threshold must be clamped[2] but does
>>> not say anything about the start threshold.
>>>
>>> In this proposal I've special-cased start==0 to actually disable the
>>> functionality via the enable bit, and otherwise made both start and
>>> end thresholds be clamped to the acceptable range. Hopefully that's
>>> fine?
>> It is fine to clamping the threshold to the acceptable range. Thank you for making the changes.
>>> Or should the [1 - 49] range for start actually be rejected?
>> The minimum charging start threshold was set to 50 to improve user experience. If the threshold is too low and the system keeps drawing power from the battery frequently due to a large system load and a weak charger, the laptop will only begin charging when the battery level falls below that threshold. If the user disconnects the charger at that time, then the device would be only having a battery below 50%. Setting the threshold at 50 ensures the battery always stays above 50%.
> So can we set it lower?
>
> Such decisions are best deferred to userspace and/or the user, which can
> limit what the kernel exposes as necessary/deemed useful
>
> Konrad
Yes, it can be set to a lower value.
However, I am still having concerns that the inappropriate start and end
threshold settings would cause a very bad user experience if they are
misused, since these thresholds are stored in nvmem and they won't be
reset until battery is unplugged or completely drained. For example, if
someone intentionally sets the start threshold to 1 and end threshold to
6, and if the laptop was shutdown with a battery SoC less than the end
threshold, I am not sure if <6% percent battery level would be good
enough to boot up the laptop successfully, if it is not, then the laptop
may not have chance to charge up until you hot plug the battery.
Also, from battery management firmware point of view, the charge control
feature was mainly designed for battery health management, to slow the
aging of Li-ion battery by preventing it from being frequently charged
to full state. Having a too low minimum start threshold setting won't
help anything on that.
Thanks
Fenglin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] power: supply: qcom_battmgr: improve charge control threshold handling
2025-11-18 2:29 ` Fenglin Wu
@ 2025-11-18 10:13 ` Konrad Dybcio
0 siblings, 0 replies; 12+ messages in thread
From: Konrad Dybcio @ 2025-11-18 10:13 UTC (permalink / raw)
To: Fenglin Wu, Val Packett, Sebastian Reichel, Neil Armstrong,
linux-arm-msm, linux-pm, linux-kernel
On 11/18/25 3:29 AM, Fenglin Wu wrote:
>
> On 11/17/2025 8:45 PM, Konrad Dybcio wrote:
>> On 11/17/25 6:12 AM, Fenglin Wu wrote:
>>> On 10/13/2025 7:32 AM, Val Packett wrote:
>>>> Currently, upowerd is unable to turn off the battery preservation mode[1]
>>>> on Qualcomm laptops, because it does that by setting the start threshold to
>>>> zero and the driver returns an error:
>>>>
>>>> pmic_glink.power-supply.0: charge control start threshold exceed range: [50 - 95]
>>>>
>>>> Kernel documentation says the end threshold must be clamped[2] but does
>>>> not say anything about the start threshold.
>>>>
>>>> In this proposal I've special-cased start==0 to actually disable the
>>>> functionality via the enable bit, and otherwise made both start and
>>>> end thresholds be clamped to the acceptable range. Hopefully that's
>>>> fine?
>>> It is fine to clamping the threshold to the acceptable range. Thank you for making the changes.
>>>> Or should the [1 - 49] range for start actually be rejected?
>>> The minimum charging start threshold was set to 50 to improve user experience. If the threshold is too low and the system keeps drawing power from the battery frequently due to a large system load and a weak charger, the laptop will only begin charging when the battery level falls below that threshold. If the user disconnects the charger at that time, then the device would be only having a battery below 50%. Setting the threshold at 50 ensures the battery always stays above 50%.
>> So can we set it lower?
>>
>> Such decisions are best deferred to userspace and/or the user, which can
>> limit what the kernel exposes as necessary/deemed useful
>>
>> Konrad
>
> Yes, it can be set to a lower value.
>
> However, I am still having concerns that the inappropriate start and end threshold settings would cause a very bad user experience if they are misused, since these thresholds are stored in nvmem and they won't be reset until battery is unplugged or completely drained. For example, if someone intentionally sets the start threshold to 1 and end threshold to 6, and if the laptop was shutdown with a battery SoC less than the end threshold, I am not sure if <6% percent battery level would be good enough to boot up the laptop successfully, if it is not, then the laptop may not have chance to charge up until you hot plug the battery.
>
> Also, from battery management firmware point of view, the charge control feature was mainly designed for battery health management, to slow the aging of Li-ion battery by preventing it from being frequently charged to full state. Having a too low minimum start threshold setting won't help anything on that.
Perhaps 50 makes sense then. Maybe we can encode this reasoning somewhere
in the power supply API docs so it's harder to make the "can't boot anymore"
mistake you mentioned
Konrad
^ permalink raw reply [flat|nested] 12+ messages in thread