Skip to content

driver: isl9241: clear BGATE_OFF when bypass entry aborts - #99

Open
0xdeafbeef wants to merge 1 commit into
FrameworkComputer:fwk-tulip-29169from
0xdeafbeef:bgate-rollback-fix
Open

0xdeafbeef wants to merge 1 commit into
FrameworkComputer:fwk-tulip-29169from
0xdeafbeef:bgate-rollback-fix

Conversation

@0xdeafbeef

Copy link
Copy Markdown

Problem

My Framework 16 (AMD Ryzen 7040, BIOS 04.05, EC lotus-4.0.5-2246d58 RO) did not charge on 65 W and 100 W USB-C supplies.
The battery sat at 8-9 % with 0-4 mA battery current, while the charger target was 17.6 V / 5488 mA.

I read the charger options with framework_tool --host-command (CHARGE_STATE GET_PARAM, the EC's copy from its last charge loop): CONTROL0=0x0060, CONTROL1=0x0343, the same in every read but one transient 0x00000000.
CONTROL1 bit 6 (BGATE force off) was set.
Per the ISL9241 datasheet this turns the battery FET off, and the charger regulates VSYS to MaxSystemVoltage with the battery isolated.

Lotus init writes CONTROL0=0x0000 and CONTROL1=0x0303.
CONTROL0 bits 5 and 6 are set only by isl9241_nvdc_to_bypass() (steps 2 and 4).
So since the last EC init (EC uptime was about 76 days) there was an NVDC -> bypass attempt that set BGATE_OFF in step 7 and did not reach step 15, which clears it.
Lotus requests bypass when the charge-manager power is above 100 W.

In isl9241_nvdc_to_bypass(), if the VSYS ADC read fails or VSYS does not reach the target within ISL9241_BYPASS_VSYS_TIMEOUT_MS, the function returns early and skips step 15.
The caller isl9241_enable_bypass_mode() then calls isl9241_bypass_to_nvdc(), which does not touch BGATE and returns success.
Nothing else clears the bit.
Dropping to 100 W or less does not call the driver again, because the charger is not in bypass.
Only EC re-init (for example BIOS "Disconnect Battery") or a later successful bypass entry clears it.

I ran the driver functions verbatim in a host harness against fake registers, starting from the Lotus init values.
After a VSYS ADC error or a timeout it ends in exactly CONTROL0=0x0060, CONTROL1=0x0343.

I do not know why the VSYS wait failed on this laptop or when the attempt happened.

Clearing only BGATE_OFF on the laptop (CHARGE_STATE SET_PARAM with 0x03030060, the same values without bit 6) made the battery charge without a reboot.
Battery current went from 0 to 2.43 A within 25 seconds, and the charge went from 478 to 500 mAh.
This is the same register change the patch makes, from the same state.

Change

When the VSYS wait in isl9241_nvdc_to_bypass() fails (ADC error or timeout), clear CONTROL1.BGATE_OFF before returning.
The rest of the rollback stays in isl9241_bypass_to_nvdc() as before.

Two new ztests in zephyr/test/drivers/isl9241 run in a new testcase drivers.isl9241.bypass (drivers.isl9241 plus CONFIG_PLATFORM_EC_CHARGER_BYPASS_MODE).
Both check that BGATE_OFF is clear after charger_enable_bypass_mode(true) aborts:

  • test_bypass_abort_releases_bgate injects a read failure on the VSYS ADC register.
    For that, the ISL9241 emulator gets isl9241_emul_get_i2c_common_data(), like the ISL923x emulator has.
  • test_bypass_timeout_releases_bgate gives the charge manager a 20 V supply on port 0 (as the override port, since it is dual-role) while the emulated VSYS stays at 0, so the wait times out.

include/driver/charger/isl9241_public.h now includes <stdbool.h>.
It declares bool isl9241_is_in_bypass_mode(), and the ISL9241 emulator includes it first; in my GCC 14 build the emulator did not compile without it.

Testing

Twister on native_sim/native/64 with host GCC 14:

  • With the fix, both tests pass, and drivers.isl9241 and drivers.isl9241.ec_host_cmd still pass.
  • Without the fix, both fail with "BGATE still forced off after aborted entry".

The test log shows both abort paths: "Aborting bypass mode. Vsys is unknown." and "Aborting bypass mode. Vsys too low (0 < 18304)", each followed by "bypass -> nvdc".

To build locally I passed -x=EXTRA_CFLAGS=-Wno-error=incompatible-pointer-types (GCC 14 rejects zephyr/emul/emul_bmi3xx.c) and -x=CONFIG_NUM_PREEMPT_PRIORITIES=40 (the BUILD_ASSERT in zephyr/shim/src/tasks.c fails with the Zephyr revision I used).
Both failures happen without this change.

I have not flashed the patched firmware or tested it on hardware.

Not covered

On this rollback path isl9241_bypass_to_nvdc() sets MaxSystemVoltage to VBUS - 256 mV for at least 1 ms, now with BGATE normal; per the datasheet the register clamps it to 18.304 V.
The regular bypass exit does the same with BGATE normal.

Only Lotus enables bypass mode.
Framework 16 with Ryzen AI 300 (tulip) uses BQ25770 without bypass, so reports from those laptops in SoftwareFirmwareIssueTracker#205 have a different cause.

isl9241_nvdc_to_bypass() sets CONTROL1.BGATE_OFF in step 7 and clears
it in step 15. If the VSYS ADC read fails or VSYS does not reach the
target within ISL9241_BYPASS_VSYS_TIMEOUT_MS, the function returns
early and skips step 15. The caller isl9241_enable_bypass_mode() then
calls isl9241_bypass_to_nvdc(), which does not touch BGATE and returns
success. The battery FET stays off and the battery is isolated.

Nothing else clears the bit. Dropping to 100 W or less does not call
the driver again, because the charger is not in bypass. Only EC
re-init or a later successful bypass entry clears it.

A Framework 16 (Ryzen 7040, EC lotus-4.0.5-2246d58) was in this state
with CONTROL0=0x0060 and CONTROL1=0x0343. It did not charge on 65 W
and 100 W USB-C supplies and sat at 8-9 % with 0-4 mA battery current.
Clearing only BGATE_OFF through CHARGE_STATE SET_PARAM made it charge
at 2.43 A within 25 seconds, without a reboot.

Clear BGATE_OFF before returning when the VSYS wait fails. The rest of
the rollback stays in isl9241_bypass_to_nvdc().

Add two tests, run in a new testcase drivers.isl9241.bypass with
CONFIG_PLATFORM_EC_CHARGER_BYPASS_MODE. Both check that BGATE_OFF is
clear after charger_enable_bypass_mode(true) aborts.
test_bypass_abort_releases_bgate injects a read failure on the VSYS
ADC register. test_bypass_timeout_releases_bgate gives the charge
manager a 20 V supply on port 0 while the emulated VSYS stays at 0,
so the wait times out. Add isl9241_emul_get_i2c_common_data() to the
ISL9241 emulator for the first one, like the ISL923x emulator has.

Include <stdbool.h> in isl9241_public.h. It declares
bool isl9241_is_in_bypass_mode(), and the ISL9241 emulator includes
it first; with GCC 14 the emulator did not compile without it.

BRANCH=fwk-tulip-29169
BUG=lotus battery stops charging after an aborted bypass mode entry
TEST=twister drivers.isl9241.bypass passes; both new tests fail
     without the fix
@0xdeafbeef

Copy link
Copy Markdown
Author

@JohnAZoidberg ptal

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