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 <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

Author: 0xdeafbeef

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);

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 <stdbool.h>
+
 #define ISL9241_ADDR_FLAGS 0x09
 
 /* Default minimum VIN voltage controlled by ISL9241_REG_VIN_VOLTAGE */

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;

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 <zephyr/drivers/emul.h>
 
+/**
+ * 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
  *

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)
 {
 	/*

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