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