From aa609ec61e728237cb8569ca8d1d67b5c16cd40e Mon Sep 17 00:00:00 2001 From: Vladimir Date: Mon, 2 Nov 2026 19:50:13 +0100 Subject: [PATCH] driver: isl9241: clear BGATE_OFF when bypass entry aborts 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 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 --- driver/charger/isl9241.c | 16 +++++- include/driver/charger/isl9241_public.h | 2 + zephyr/emul/emul_isl9241.c | 6 ++ zephyr/include/emul/emul_isl9241.h | 11 ++++ zephyr/test/drivers/isl9241/src/isl9241.c | 68 +++++++++++++++++++++++ zephyr/test/drivers/testcase.yaml | 10 ++++ 6 files changed, 111 insertions(+), 2 deletions(-) diff --git a/driver/charger/isl9241.c b/driver/charger/isl9241.c index 247260c1d0..1cd8723e86 100644 --- a/driver/charger/isl9241.c +++ b/driver/charger/isl9241.c @@ -911,16 +911,28 @@ static enum ec_error_list isl9241_nvdc_to_bypass(int chgnum) if (_get_vsys_voltage(chgnum, 0, &vsys)) { CPRINTS("Aborting bypass mode. Vsys is unknown."); rv = EC_ERROR_UNKNOWN; - goto unlock_control3; + break; } if (timestamp_expired(deadline, NULL)) { CPRINTS("Aborting bypass mode. Vsys too low (%d < %d)", vsys, vsys_target); rv = EC_ERROR_TIMEOUT; - goto unlock_control3; + break; } } while (vsys < vsys_target - 256); + if (rv) { + /* + * Undo step 7. The caller's rollback to NVDC does not touch + * BGATE, so the battery would stay disconnected and never + * charge until the charger is re-initialized or a later + * bypass entry succeeds. + */ + isl9241_update(chgnum, ISL9241_REG_CONTROL1, + ISL9241_CONTROL1_BGATE_OFF, MASK_CLR); + goto unlock_control3; + } + /* 10*: Turn on Bypass gate */ isl9241_update(chgnum, ISL9241_REG_CONTROL0, ISL9241_CONTROL0_EN_BYPASS_GATE, MASK_SET); diff --git a/include/driver/charger/isl9241_public.h b/include/driver/charger/isl9241_public.h index 19b5af6297..1116797811 100644 --- a/include/driver/charger/isl9241_public.h +++ b/include/driver/charger/isl9241_public.h @@ -8,6 +8,8 @@ #ifndef __CROS_EC_DRIVER_CHARGER_ISL9241_PUBLIC_H #define __CROS_EC_DRIVER_CHARGER_ISL9241_PUBLIC_H +#include + #define ISL9241_ADDR_FLAGS 0x09 /* Default minimum VIN voltage controlled by ISL9241_REG_VIN_VOLTAGE */ diff --git a/zephyr/emul/emul_isl9241.c b/zephyr/emul/emul_isl9241.c index 2b41230e5b..bc71fb49b9 100644 --- a/zephyr/emul/emul_isl9241.c +++ b/zephyr/emul/emul_isl9241.c @@ -73,6 +73,12 @@ uint16_t isl9241_emul_peek(const struct emul *emul, int reg) return data->regs[reg]; } +struct i2c_common_emul_data * +isl9241_emul_get_i2c_common_data(const struct emul *emul) +{ + return emul->data; +} + void isl9241_emul_set_vbus(const struct emul *emul, int vbus_mv) { struct isl9241_emul_data *data = (struct isl9241_emul_data *)emul->data; diff --git a/zephyr/include/emul/emul_isl9241.h b/zephyr/include/emul/emul_isl9241.h index 2838687731..d40aec7df4 100644 --- a/zephyr/include/emul/emul_isl9241.h +++ b/zephyr/include/emul/emul_isl9241.h @@ -6,8 +6,19 @@ #ifndef EMUL_ISL9241_H #define EMUL_ISL9241_H +#include "emul/emul_common_i2c.h" + #include +/** + * Get the I2C common data, e.g. to inject register read failures + * + * @param emul - ISL9241 emulator data + * @return I2C common emulator data + */ +struct i2c_common_emul_data * +isl9241_emul_get_i2c_common_data(const struct emul *emul); + /** * Peek an internal register value * diff --git a/zephyr/test/drivers/isl9241/src/isl9241.c b/zephyr/test/drivers/isl9241/src/isl9241.c index a39d7ecda8..3580d37fa2 100644 --- a/zephyr/test/drivers/isl9241/src/isl9241.c +++ b/zephyr/test/drivers/isl9241/src/isl9241.c @@ -4,6 +4,7 @@ */ #include "battery_smart.h" +#include "charge_manager.h" #include "charger.h" #include "console.h" #include "driver/charger/isl9241.h" @@ -252,6 +253,73 @@ ZTEST_F(isl9241_driver, test_dump_registers) } } +/* + * An NVDC -> bypass entry that aborted while waiting for VSYS left + * CONTROL1.BGATE_OFF set, so the battery stayed disconnected on AC. + */ +ZTEST_F(isl9241_driver, test_bypass_abort_releases_bgate) +{ + struct i2c_common_emul_data *common_data = + isl9241_emul_get_i2c_common_data(fixture->isl9241_emul); + uint16_t control1; + + if (!IS_ENABLED(CONFIG_PLATFORM_EC_CHARGER_BYPASS_MODE)) + ztest_test_skip(); + + /* Make NVDC -> bypass abort while it waits for VSYS. */ + i2c_common_emul_set_read_fail_reg(common_data, + ISL9241_REG_VSYS_ADC_RESULTS); + charger_enable_bypass_mode(CHARGER_SOLO, true); + i2c_common_emul_set_read_fail_reg(common_data, + I2C_COMMON_EMUL_NO_FAIL_REG); + + control1 = isl9241_emul_peek(fixture->isl9241_emul, + ISL9241_REG_CONTROL1); + zassert_false(control1 & ISL9241_CONTROL1_BGATE_OFF, + "BGATE still forced off after aborted entry: 0x%04x", + control1); +} + +/* Same as above, but the entry aborted because VSYS never reached target. */ +ZTEST_F(isl9241_driver, test_bypass_timeout_releases_bgate) +{ + struct charge_port_info charge = { .current = 3000, .voltage = 20000 }; + struct charge_port_info none = { 0 }; + uint16_t control0, control1; + + if (!IS_ENABLED(CONFIG_PLATFORM_EC_CHARGER_BYPASS_MODE)) + ztest_test_skip(); + + /* VSYS stays at 0 while the target follows the 20 V supply. */ + k_msleep(1000); + charge_manager_update_charge(CHARGE_SUPPLIER_TYPEC, USBC_PORT_C0, + &charge); + /* Port 0 is dual-role, so it charges only as the override port. */ + zassert_ok(charge_manager_set_override(USBC_PORT_C0)); + k_msleep(500); + zassert_equal(charge_manager_get_charger_voltage(), charge.voltage, + "port %d supplier %d", + charge_manager_get_active_charge_port(), + charge_manager_get_supplier()); + isl9241_emul_set_vsys(fixture->isl9241_emul, 0); + + charger_enable_bypass_mode(CHARGER_SOLO, true); + charge_manager_set_override(OVERRIDE_OFF); + charge_manager_update_charge(CHARGE_SUPPLIER_TYPEC, USBC_PORT_C0, + &none); + k_msleep(500); + + control0 = isl9241_emul_peek(fixture->isl9241_emul, + ISL9241_REG_CONTROL0); + control1 = isl9241_emul_peek(fixture->isl9241_emul, + ISL9241_REG_CONTROL1); + zassert_false(control0 & ISL9241_CONTROL0_EN_BYPASS_GATE, + "Bypass entry did not abort: 0x%04x", control0); + zassert_false(control1 & ISL9241_CONTROL1_BGATE_OFF, + "BGATE still forced off after aborted entry: 0x%04x", + control1); +} + ZTEST(isl9241_driver, test_prochot_dump) { /* diff --git a/zephyr/test/drivers/testcase.yaml b/zephyr/test/drivers/testcase.yaml index 765f81f1c1..53736c74a3 100644 --- a/zephyr/test/drivers/testcase.yaml +++ b/zephyr/test/drivers/testcase.yaml @@ -741,6 +741,16 @@ tests: extra_dtc_overlay_files: - ./boards/native_sim.overlay - ./isl9241/usbc.dts + drivers.isl9241.bypass: + extra_configs: + - CONFIG_LINK_TEST_SUITE_ISL9241=y + - CONFIG_PLATFORM_EC_CHARGER_DUMP_PROCHOT=y + - CONFIG_PLATFORM_EC_CONSOLE_CMD_CHARGER_DUMP=y + - CONFIG_PLATFORM_EC_CHARGER_ISL9241=y + - CONFIG_PLATFORM_EC_CHARGER_BYPASS_MODE=y + extra_dtc_overlay_files: + - ./boards/native_sim.overlay + - ./isl9241/usbc.dts drivers.isl9241.ec_host_cmd: extra_configs: - CONFIG_LINK_TEST_SUITE_ISL9241=y