chgstv2: Refactor charger_discharge_on_ac

This patch makes charger_discharge_on_ac call board_discharge_on_ac.
It also makes set_chg_ctrl_mode call charger_discharge_on_ac. This
makes sense since when the charge control mode changes,
discharge-on-ac also needs to be enabled or disabled.

BUG=b:188457962
BRANCH=none
TEST=make runhosttests

Cq-Depend: chromium:3107851
Change-Id: I65ec09f580afc987cc86f4c60c15c1f90ead6c3c
Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2986848
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3105739
Reviewed-by: caveh jalali <caveh@chromium.org>

5 files changed, 96 insertions, 31 deletions

diff --git a/common/charge_state_v2.c b/common/charge_state_v2.c
index 5027b719ba..ef12f0bf15 100644
--- a/common/charge_state_v2.c
+++ b/common/charge_state_v2.c
@@ -1290,23 +1290,45 @@ void chgstate_set_manual_voltage(int volt_mv)
 /* Force charging off before the battery is full. */
 static int set_chg_ctrl_mode(enum ec_charge_control_mode mode)
 {
+	bool discharge_on_ac = false;
+	int current, voltage;
+	int rv;
+
+	current = manual_current;
+	voltage = manual_voltage;
+
+	if (mode >= CHARGE_CONTROL_COUNT)
+		return EC_ERROR_INVAL;
+
 	if (mode == CHARGE_CONTROL_NORMAL) {
-		chg_ctl_mode = mode;
-		manual_current = -1;
-		manual_voltage = -1;
+		current = -1;
+		voltage = -1;
 	} else {
-		/*
-		 * Changing mode is only meaningful if external power is
-		 * present. If it's not present we can't charge anyway.
-		 */
+		/* Changing mode is only meaningful if AC is present. */
 		if (!curr.ac)
 			return EC_ERROR_NOT_POWERED;
 
-		chg_ctl_mode = mode;
-		manual_current = 0;
-		manual_voltage = 0;
+		if (mode == CHARGE_CONTROL_DISCHARGE) {
+			if (!IS_ENABLED(CONFIG_CHARGER_DISCHARGE_ON_AC))
+				return EC_ERROR_UNIMPLEMENTED;
+			discharge_on_ac = true;
+		} else if (mode == CHARGE_CONTROL_IDLE) {
+			current = 0;
+			voltage = 0;
+		}
+	}
+
+	if (IS_ENABLED(CONFIG_CHARGER_DISCHARGE_ON_AC)) {
+		rv = charger_discharge_on_ac(discharge_on_ac);
+		if (rv != EC_SUCCESS)
+			return rv;
 	}
 
+	/* Commit all atomically */
+	chg_ctl_mode = mode;
+	manual_current = current;
+	manual_voltage = voltage;
+
 	return EC_SUCCESS;
 }
 
@@ -2349,16 +2371,6 @@ static int charge_command_charge_control(struct host_cmd_handler_args *args)
 	if (rv != EC_SUCCESS)
 		return EC_RES_ERROR;
 
-#ifdef CONFIG_CHARGER_DISCHARGE_ON_AC
-#ifdef CONFIG_CHARGER_DISCHARGE_ON_AC_CUSTOM
-	rv = board_discharge_on_ac(p->mode == CHARGE_CONTROL_DISCHARGE);
-#else
-	rv = charger_discharge_on_ac(p->mode == CHARGE_CONTROL_DISCHARGE);
-#endif
-	if (rv != EC_SUCCESS)
-		return EC_RES_ERROR;
-#endif
-
 	return EC_RES_SUCCESS;
 }
 DECLARE_HOST_COMMAND(EC_CMD_CHARGE_CONTROL, charge_command_charge_control,
@@ -2558,7 +2570,6 @@ static int command_chgstate(int argc, char **argv)
 						CHARGE_CONTROL_NORMAL);
 			if (rv)
 				return rv;
-#ifdef CONFIG_CHARGER_DISCHARGE_ON_AC
 		} else if (!strcasecmp(argv[1], "discharge")) {
 			if (argc <= 2)
 				return EC_ERROR_PARAM_COUNT;
@@ -2568,14 +2579,6 @@ static int command_chgstate(int argc, char **argv)
 						CHARGE_CONTROL_NORMAL);
 			if (rv)
 				return rv;
-#ifdef CONFIG_CHARGER_DISCHARGE_ON_AC_CUSTOM
-			rv = board_discharge_on_ac(val);
-#else
-			rv = charger_discharge_on_ac(val);
-#endif /* CONFIG_CHARGER_DISCHARGE_ON_AC_CUSTOM */
-			if (rv)
-				return rv;
-#endif /* CONFIG_CHARGER_DISCHARGE_ON_AC */
 		} else if (!strcasecmp(argv[1], "debug")) {
 			if (argc <= 2)
 				return EC_ERROR_PARAM_COUNT;
diff --git a/common/charger.c b/common/charger.c
index f43ada4ff2..585fdc3a68 100644
--- a/common/charger.c
+++ b/common/charger.c
@@ -41,6 +41,15 @@ static void dptf_disable_hook(void)
 DECLARE_HOOK(HOOK_CHIPSET_SUSPEND, dptf_disable_hook, HOOK_PRIO_DEFAULT);
 DECLARE_HOOK(HOOK_CHIPSET_SHUTDOWN, dptf_disable_hook, HOOK_PRIO_DEFAULT);
 
+/*
+ * Boards should override this function if their count may vary during run-time
+ * due to different DB options.
+ */
+__overridable uint8_t board_get_charger_chip_count(void)
+{
+	return CHARGER_NUM;
+}
+
 int charger_closest_voltage(int voltage)
 {
 	const struct charger_info *info = charger_get_info();
@@ -213,3 +222,24 @@ static int command_charger(int argc, char **argv)
 DECLARE_CONSOLE_COMMAND(charger, command_charger,
 			"[input | current | voltage | dptf] [newval]",
 			"Get or set charger param(s)");
+
+enum ec_error_list charger_discharge_on_ac(int enable)
+{
+	int chgnum;
+	int rv = EC_ERROR_UNIMPLEMENTED;
+
+	if (IS_ENABLED(CONFIG_CHARGER_DISCHARGE_ON_AC_CUSTOM))
+		return board_discharge_on_ac(enable);
+
+	/*
+	 * When discharge on AC is selected, cycle through all chargers to
+	 * enable or disable this feature.
+	 */
+	for (chgnum = 0; chgnum < board_get_charger_chip_count(); chgnum++) {
+		if (chg_chips[chgnum].drv->discharge_on_ac)
+			rv = chg_chips[chgnum].drv->discharge_on_ac(chgnum,
+								    enable);
+	}
+
+	return rv;
+}
diff --git a/driver/charger/isl923x.c b/driver/charger/isl923x.c
index 89fd3ad1c2..24e8ae42f4 100644
--- a/driver/charger/isl923x.c
+++ b/driver/charger/isl923x.c
@@ -394,7 +394,7 @@ init_fail:
 }
 DECLARE_HOOK(HOOK_INIT, isl923x_init, HOOK_PRIO_INIT_I2C + 1);
 
-int charger_discharge_on_ac(int enable)
+static enum ec_error_list isl923x_discharge_on_ac(int chgnum, int enable)
 {
 	int rv;
 	int control1;
@@ -703,3 +703,7 @@ static int command_isl923x_dump(int argc, char **argv)
 DECLARE_CONSOLE_COMMAND(charger_dump, command_isl923x_dump, "",
 			"Dumps ISL923x registers");
 #endif /* CONFIG_CMD_CHARGER_DUMP */
+
+const struct charger_drv isl923x_drv = {
+	.discharge_on_ac = isl923x_discharge_on_ac,
+};
diff --git a/driver/charger/isl923x.h b/driver/charger/isl923x.h
index 118f22b77d..e3ae1bfb0e 100644
--- a/driver/charger/isl923x.h
+++ b/driver/charger/isl923x.h
@@ -298,3 +298,5 @@ enum isl9237_fsm_state {
 
 #define I2C_ADDR_CHARGER ISL923X_ADDR
 #endif /* __CROS_EC_ISL923X_H */
+
+extern const struct charger_drv isl923x_drv;
diff --git a/include/charger.h b/include/charger.h
index 964c3bf759..9a22d6b33f 100644
--- a/include/charger.h
+++ b/include/charger.h
@@ -40,6 +40,32 @@ struct charger_params {
 	int flags;
 };
 
+struct charger_drv {
+	/* Discharge battery when on AC power. */
+	enum ec_error_list (*discharge_on_ac)(int chgnum, int enable);
+};
+
+struct charger_config_t {
+	int i2c_port;
+	uint16_t i2c_addr_flags;
+	const struct charger_drv *drv;
+};
+
+extern const struct charger_config_t chg_chips[];
+
+__override_proto uint8_t board_get_charger_chip_count(void);
+
+/*
+ * Note: CHARGER_SOLO should be used anywhere the charger index being called is
+ * only valid for a single-chip system.  This will then generate build errors if
+ * the callsite is compliled for a multi-chip system, which needs to re-evaluate
+ * the charger index to act upon.
+ */
+enum chg_id {
+	CHARGER_SOLO,
+	CHARGER_NUM,
+};
+
 /* Get the current charger_params. Failures are reported in .flags */
 void charger_get_params(struct charger_params *chg);
 
@@ -123,7 +149,7 @@ int charger_get_voltage(int *voltage);
 int charger_set_voltage(int voltage);
 
 /* Discharge battery when on AC power. */
-int charger_discharge_on_ac(int enable);
+enum ec_error_list charger_discharge_on_ac(int enable);
 
 /* Get the VBUS voltage (mV) from the charger */
 int charger_get_vbus_voltage(int port);