Skip to content

fwk: reapply an explicitly requested battery charge limit - #100

Open
thelogicmatrix wants to merge 1 commit into
FrameworkComputer:fwk-tulip-29169from
thelogicmatrix:fix/charge-limit-source-checks
Open

thelogicmatrix wants to merge 1 commit into
FrameworkComputer:fwk-tulip-29169from
thelogicmatrix:fix/charge-limit-source-checks

Conversation

@thelogicmatrix

Copy link
Copy Markdown

An explicit charge-limit SET can succeed without restoring the requested sustainer thresholds. battery_percentage_control() compares the requested percentage with old_charger_limit, but another control path can disable or replace the live sustainer without changing that cache. Re-sending the same percentage then leaves the live configuration unchanged.

Invalidate the cached percentage when CHG_LIMIT_SET_LIMIT accepts a value in BATT_EXTENDER_STAGE_0. The existing call to battery_percentage_control() then reapplies the requested static range, even if the stored percentage is unchanged. Active Battery Extender stages keep their existing caps. The periodic hook and GET command keep their existing behavior. The patch does not periodically reassert static thresholds over a separate host charge-control policy.

This addresses the same-value SET defect. It does not establish which path originally changed the sustainer on the affected laptop, and it does not claim to repair fuel-gauge capacity estimates or all charge-limit failures.

Related field evidence is in SoftwareFirmwareIssueTracker#284. On a Laptop 16 Ryzen 7040 running BIOS 04.05 and EC lotus-4.0.5-2246d58, re-sending 80 left the battery reported as charging at 96%. Changing the limit through 100, 60 and 80 changed the battery status to discharging. Archived samples show charging at 22:31:05 and discharging at 22:31:26 on 2026-10-22, Singapore time. A separate watcher recorded 95/80 before recovery and 81/80 afterward.

Two corrections to the issue's original interpretation matter here. The sustainer already handles a percentage above its upper threshold, so a percentage jump does not by itself bypass that check. Also, the tool's Charger Current comes from the charger's programmed current-limit register. It stayed at 2744 mA after the battery status changed to discharging, so it is not evidence of 2744 mA flowing into the battery. The event that changed the live sustainer configuration remains unknown.

Validation:

  • The host regression compiles the complete production battery_extender.c translation unit and the real protocol headers, with EC services mocked.
  • Before the patch, same-value SET fails to restore both disabled thresholds and thresholds replaced by another policy. After the patch, both cases pass.
  • The other cases model a separate host charge-control policy by disabling the sustainer and verify that a timer tick and GET leave it disabled. They also verify that both active extender stages retain their caps when the same limit is re-sent, that invalid low limits are rejected, and that 100 followed by 80 still works. All six tests pass.
  • Four additional cases cover extender deadlines and AC reset, maximum configuration intervals, console time units, and console enable/disable/timer controls. All ten tests pass through the existing util/run_tests.sh.
  • The source uses Zephyr time units and the repository formatter. Minute/hour values remain 64-bit, which also fixes overflow of an accepted 9999-minute reset interval. That interval fails the final regression on the unmodified source.
  • GCC executes 51 of 56 changed executable C lines, or 91.1%, including both outcomes of the recovery guard. This includes source formatting and time-unit migration. It is not a whole-file coverage claim.
  • Unmodified and patched lotus RO/RW images both build with Framework Zephyr e414f3a266c2c83ef7fbde738cbe1990c12e11d6, CMSIS ada89e4063a02bf12797415ebab9d21f0cfaeef1, and Zephyr SDK 0.17.2. The build reports existing devicetree warnings in both versions.
  • No firmware was flashed. This is host-regression and build validation, not a hardware validation claim.

All 50 Chromium preupload checks pass on the complete consolidated change, including copyright, legacy-symbol, formatting and lint checks. The check runner uses an unprivileged account with the required Zephyr checkout and tool dependencies.

The inherited legacy board-fleet checks remain a validation limit. make buildall requires Chromium's multi-architecture coreboot SDK, which this environment lacks. The legacy host suite with the available native compiler stops on the unchanged mallinfo() call in common/shared_mem_libc.c, which Debian 12 treats as a deprecated declaration with -Werror. Those separate build/test suites are not covered by the passing preupload result.

Run the regression with Python 3.8+ and a C compiler on PATH:

python3 util/test_framework_battery_extender.py

Set CC to select a different compiler. With GCC and gcov, export coverage using --coverage-dir /tmp/ec-coverage.

AI disclosure: this patch and its regression tests were prepared with AI coding assistance. The executed validation and remaining limitations are listed above.

Cherry-pick status:

Branch PR
fwk-tulip-29169 This PR
fwk-main Not proposed, behavior not assessed
fwk-hx20-hx30-4410 Not proposed, behavior not assessed
fwk-lotus-azalea-19573 Not proposed, behavior not assessed
fwk-marigold-22606 Not proposed, behavior not assessed
fwk-sunflower-26784 Not proposed, behavior not assessed
fwk-dogwood-27111 Not proposed, behavior not assessed
fwk-lilac-27116 Not proposed, behavior not assessed

Another control path can disable or replace the live sustainer while the
cached requested percentage stays unchanged. Re-sending that percentage
then succeeds without restoring the requested static thresholds.

Invalidate the cache on an accepted explicit SET in stage 0. Preserve the
caps of active Battery Extender stages and leave timer/GET behavior alone.
Add a host regression to the existing utility runner and export optional
GCC coverage.

Use Zephyr time units and the repository formatter to pass source checks.
Keep minute/hour arithmetic 64-bit so the accepted 9999-minute reset
interval is not truncated. Exercise extender deadlines, configuration
round-trips and console timer controls as well as charge-limit recovery.

Related: FrameworkComputer/SoftwareFirmwareIssueTracker#284

BUG=None
TEST=python3 util/test_framework_battery_extender.py (10 pass)
TEST=bash util/run_tests.sh (pass)
TEST=lotus RO/RW build with Zephyr SDK 0.17.2 (pass)
Change-Id: I3d0946955ea6b9c401674b42ae65ded7e531dc54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant