chg_ramp: Don't change ICL until vals are init'd.

When the charge ramp task starts, it starts up in the DISCONNECTED
state.  From this state, it's possible to set the input current limit to
0 mA.  However, upon task start, we shouldn't take any action until we
have valid values from a new charge supplier or a supplier leaving.

This commit changes the charge ramp task to not touch the input current
limit until the charge ramp state changes or the desired input current
changes at least once.

BUG=b:36468002
BRANCH=gru,master
TEST=Flash kevin EC RW or build AP FW that contains the fix that will by
sync'd by EC SW sync.  Make sure WP is asserted to prevent PD
communications in RO.  Unplug battery.  Plug in AC, verify that system
can boot up to UI on AC alone.
TEST=make -j buildall

Change-Id: I351917bce7902c49d1bb842a0cc83dd161d75b6f
Reviewed-on: https://chromium-review.googlesource.com/461382
Commit-Queue: Aseda Aboagye <aaboagye@chromium.org>
Tested-by: Aseda Aboagye <aaboagye@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/463927
Commit-Ready: Aseda Aboagye <aaboagye@chromium.org>
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>

2 files changed, 27 insertions, 6 deletions

diff --git a/common/charge_ramp.c b/common/charge_ramp.c
index 1c9dfecc33..a5928983fc 100644
--- a/common/charge_ramp.c
+++ b/common/charge_ramp.c
@@ -159,6 +159,7 @@ void chg_ramp_task(void)
 	static enum chg_ramp_state ramp_st_prev = CHG_RAMP_DISCONNECTED,
 				   ramp_st_new = CHG_RAMP_DISCONNECTED;
 	int active_icl_new;
+	static uint8_t values_have_changed_at_least_once;
 
 	/* Clear last OCP supplier to guarantee we ramp on first connect */
 	for (i = 0; i < CONFIG_USB_PD_PORT_COUNT; i++)
@@ -322,19 +323,28 @@ void chg_ramp_task(void)
 			task_wait_time = STABLE_VBUS_MONITOR_INTERVAL;
 			break;
 		}
-		if (ramp_st != ramp_st_new || active_icl != active_icl_new)
+		if (ramp_st != ramp_st_new || active_icl != active_icl_new) {
 			CPRINTS("Ramp p%d st%d %dmA %dmA",
 				active_port, ramp_st_new, min_icl,
 				active_icl_new);
+			values_have_changed_at_least_once = 1;
+		}
 
 		ramp_st_prev = ramp_st;
 		ramp_st = ramp_st_new;
 		active_icl = active_icl_new;
 
-		/* Set the input current limit */
-		lim = chg_ramp_get_current_limit();
-		board_set_charge_limit(active_port, active_sup, lim,
-				       lim, active_vtg);
+		/*
+		 * Don't perform any action unless something has changed.
+		 * Otherwise, when the task starts, we may try and set a current
+		 * limit that's invalid/uninitialized.
+		 */
+		if (values_have_changed_at_least_once) {
+			/* Set the input current limit */
+			lim = chg_ramp_get_current_limit();
+			board_set_charge_limit(active_port, active_sup, lim,
+					       lim, active_vtg);
+		}
 
 		if (ramp_st == CHG_RAMP_STABILIZE)
 			/*
diff --git a/test/charge_ramp.c b/test/charge_ramp.c
index 6e9f239101..29383ad133 100644
--- a/test/charge_ramp.c
+++ b/test/charge_ramp.c
@@ -121,7 +121,13 @@ static int test_no_ramp(void)
 	system_load_current_ma = 3000;
 	/* A powerful charger, but hey, you're not allowed to ramp! */
 	plug_charger(CHARGE_SUPPLIER_TEST1, 0, 500, 3000, 3000);
-	usleep(CHARGE_DETECT_DELAY_TEST);
+	/*
+	 * NOTE: Since this is currently the first test being run, give the
+	 * charge ramp task enough time to actually transition states and set
+	 * the charge limit.  This just needs at least transition to the
+	 * CHG_RAMP_OVERCURRENT_DETECT state.
+	 */
+	usleep(CHARGE_DETECT_DELAY_TEST + 200*MSEC);
 	/* That's right. Start at 500 mA */
 	TEST_ASSERT(charge_limit_ma == 500);
 	TEST_ASSERT(wait_stable_no_overcurrent());
@@ -485,6 +491,11 @@ void run_test(void)
 {
 	test_reset();
 
+	/*
+	 * If the following test order changes, make sure to add enough time for
+	 * the charge ramp task to make its first transition after plugging in a
+	 * charger.  See the comment in test_no_ramp().
+	 */
 	RUN_TEST(test_no_ramp);
 	RUN_TEST(test_full_ramp);
 	RUN_TEST(test_vbus_dip);