charge_manager: Wait for dualrole determination before charging

If a dual-role charger is plugged, we will not realize it is dual-role
until after we see a type-C charge source. It can cause us to briefly
charge from a dual-role charger, which has bad side effects related to
charge override and the lightbar. Fix this by not charging from a port
until we are fairly certain that it is a dedicated charger (based upon
PD discovery timeout).

BUG=chrome-os-partner:36390
TEST=Manual on Samus. Insert 1A Apple charger, verify correct detection.
Run 'chgoverride -2' to prevent charging, then repeatedly insert +
remove a dual-role charger on the other charge port. Verify that
charging is still prevented. Finally, insert a dedicated charger and
verify that the override is removed. Also, pass unit tests and verify
correct detection in various scenarios with various chargers.
BRANCH=Samus

Change-Id: Ia4154f34dd0a850b6e72bebadbd938f034532f14
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/247130
Reviewed-by: Alec Berg <alecaberg@chromium.org>

1 files changed, 139 insertions, 56 deletions

diff --git a/test/charge_manager.c b/test/charge_manager.c
index b0c9423632..977f516496 100644
--- a/test/charge_manager.c
+++ b/test/charge_manager.c
@@ -34,14 +34,9 @@ static unsigned int active_charge_limit = CHARGE_SUPPLIER_NONE;
 static unsigned int active_charge_port = CHARGE_PORT_NONE;
 static unsigned int charge_port_to_reject = CHARGE_PORT_NONE;
 static int new_power_request[PD_PORT_COUNT];
-static int dual_role_capable[PD_PORT_COUNT];
+static int dual_role_capable[PD_PORT_COUNT] = { CAP_DEDICATED, CAP_DEDICATED };
 static int power_role[PD_PORT_COUNT];
 
-enum {
-	DEDICATED_CHARGER = 0,
-	DUAL_ROLE_CHARGER = 1,
-};
-
 /* Callback functions called by CM on state change */
 void board_set_charge_limit(int charge_ma)
 {
@@ -85,12 +80,12 @@ static void clear_new_power_requests(void)
  * does not change dynamically, and thus won't trigger a charge manager
  * refresh, in test code and in production.
  */
-static void set_charger_role(int port, int role)
+static void set_charger_role(int port, enum dualrole_capabilities cap)
 {
-	dual_role_capable[port] = role;
+	dual_role_capable[port] = cap;
 }
 
-int pd_get_partner_dualrole_capable(int port)
+enum dualrole_capabilities pd_get_partner_dualrole_capable(int port)
 {
 	return dual_role_capable[port];
 }
@@ -130,10 +125,10 @@ static void initialize_charge_table(int current, int voltage, int ceil)
 
 	for (i = 0; i < PD_PORT_COUNT; ++i) {
 		charge_manager_set_ceil(i, ceil);
-		set_charger_role(i, DEDICATED_CHARGER);
+		set_charger_role(i, CAP_DEDICATED);
 		pd_set_role(i, PD_ROLE_SINK);
 		for (j = 0; j < CHARGE_SUPPLIER_COUNT; ++j)
-			charge_manager_update(j, i, &charge);
+			charge_manager_update_charge(j, i, &charge);
 	}
 	wait_for_charge_manager_refresh();
 }
@@ -157,7 +152,7 @@ static int test_initialization(void)
 			if (i == CHARGE_SUPPLIER_COUNT - 1 &&
 			    j == PD_PORT_COUNT - 1)
 				break;
-			charge_manager_update(i, j, &charge);
+			charge_manager_update_charge(i, j, &charge);
 		}
 
 	/* Verify no active charge port, since all pairs haven't updated */
@@ -165,9 +160,9 @@ static int test_initialization(void)
 	TEST_ASSERT(active_charge_port == CHARGE_PORT_NONE);
 
 	/* Update last pair and verify a charge port has been selected */
-	charge_manager_update(CHARGE_SUPPLIER_COUNT-1,
-			      PD_PORT_COUNT-1,
-			      &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_COUNT-1,
+				     PD_PORT_COUNT-1,
+				     &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port != CHARGE_PORT_NONE);
 
@@ -188,9 +183,9 @@ static int test_priority(void)
 	 */
 	charge.current = 2000;
 	charge.voltage = 5000;
-	charge_manager_update(CHARGE_SUPPLIER_TEST6, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST6, 0, &charge);
 	charge.current = 1000;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 1000);
@@ -200,7 +195,7 @@ static int test_priority(void)
 	 * lower charge.
 	 */
 	charge.current = 1500;
-	charge_manager_update(CHARGE_SUPPLIER_TEST7, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST7, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 1000);
@@ -210,20 +205,20 @@ static int test_priority(void)
 	 * which happens to be a different port.
 	 */
 	charge.current = 0;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);
 	TEST_ASSERT(active_charge_limit == 2000);
 
 	/* Add a charge at equal priority and verify highest charge selected */
 	charge.current = 2500;
-	charge_manager_update(CHARGE_SUPPLIER_TEST5, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST5, 0, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);
 	TEST_ASSERT(active_charge_limit == 2500);
 
 	charge.current = 3000;
-	charge_manager_update(CHARGE_SUPPLIER_TEST6, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST6, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 3000);
@@ -234,7 +229,7 @@ static int test_priority(void)
 	 * selected as the tiebreaker.
 	 */
 	charge.current = 3000;
-	charge_manager_update(CHARGE_SUPPLIER_TEST6, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST6, 0, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 3000);
@@ -268,9 +263,9 @@ static int test_charge_ceil(void)
 	/* Verify that ceiling is ignored in determining active charge port */
 	charge.current = 2000;
 	charge.voltage = 5000;
-	charge_manager_update(0, 0, &charge);
+	charge_manager_update_charge(0, 0, &charge);
 	charge.current = 2500;
-	charge_manager_update(0, 1, &charge);
+	charge_manager_update_charge(0, 1, &charge);
 	charge_manager_set_ceil(1, 750);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
@@ -294,7 +289,7 @@ static int test_new_power_request(void)
 	/* Charge from port 1 and verify NPR on port 1 only */
 	charge.current = 1000;
 	charge.voltage = 5000;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(new_power_request[0] == 0);
 	TEST_ASSERT(new_power_request[1] == 1);
@@ -309,14 +304,14 @@ static int test_new_power_request(void)
 
 	/* Change port 1 voltage and verify NPR on port 1 */
 	charge.voltage = 4000;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(new_power_request[0] == 0);
 	TEST_ASSERT(new_power_request[1] == 1);
 	clear_new_power_requests();
 
 	/* Add low-priority source and verify no NPRs */
-	charge_manager_update(CHARGE_SUPPLIER_TEST6, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST6, 0, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(new_power_request[0] == 0);
 	TEST_ASSERT(new_power_request[1] == 0);
@@ -326,7 +321,7 @@ static int test_new_power_request(void)
 	 * Add higher-priority source and verify NPR on both ports,
 	 * since we're switching charge ports.
 	 */
-	charge_manager_update(CHARGE_SUPPLIER_TEST1, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST1, 0, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(new_power_request[0] == 1);
 	TEST_ASSERT(new_power_request[1] == 1);
@@ -348,8 +343,8 @@ static int test_override(void)
 	 */
 	charge.current = 500;
 	charge.voltage = 5000;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 0, &charge);
-	charge_manager_update(CHARGE_SUPPLIER_TEST1, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST1, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 500);
@@ -369,14 +364,14 @@ static int test_override(void)
 	 * is again selected.
 	 */
 	charge.current = 0;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
 	charge_manager_set_override(0);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 
 	/* Set non-zero charge on port 0 and verify override was auto-removed */
 	charge.current = 250;
-	charge_manager_update(CHARGE_SUPPLIER_TEST5, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST5, 0, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 
@@ -385,13 +380,13 @@ static int test_override(void)
 	 * priority on the override port.
 	 */
 	charge.current = 300;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
 	charge_manager_set_override(0);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);
 	TEST_ASSERT(active_charge_limit == 300);
 	charge.current = 100;
-	charge_manager_update(CHARGE_SUPPLIER_TEST1, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST1, 0, &charge);
 	charge_manager_set_override(0);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);
@@ -403,12 +398,12 @@ static int test_override(void)
 	 * is successful.
 	 */
 	charge_manager_set_override(OVERRIDE_DONT_CHARGE);
-	set_charger_role(0, DUAL_ROLE_CHARGER);
+	set_charger_role(0, CAP_DUALROLE);
 	pd_set_role(0, PD_ROLE_SOURCE);
 	charge_manager_set_override(0);
 	wait_for_charge_manager_refresh();
 	charge.current = 200;
-	charge_manager_update(CHARGE_SUPPLIER_TEST1, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST1, 0, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);
 	TEST_ASSERT(active_charge_limit == 200);
@@ -423,7 +418,7 @@ static int test_override(void)
 
 	/* Update a charge supplier, verify that we still aren't charging */
 	charge.current = 200;
-	charge_manager_update(CHARGE_SUPPLIER_TEST1, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST1, 0, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == CHARGE_PORT_NONE);
 	TEST_ASSERT(active_charge_limit == 0);
@@ -450,10 +445,10 @@ static int test_dual_role(void)
 	 * Mark P0 as dual-role and set a charge. Verify that we don't charge
 	 * from the port.
 	 */
-	set_charger_role(0, DUAL_ROLE_CHARGER);
+	set_charger_role(0, CAP_DUALROLE);
 	charge.current = 500;
 	charge.voltage = 5000;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == CHARGE_PORT_NONE);
 	TEST_ASSERT(active_charge_limit == 0);
@@ -475,7 +470,7 @@ static int test_dual_role(void)
 	/* Mark P0 as the override port, verify that we again charge. */
 	charge_manager_set_override(0);
 	charge.current = 550;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);
 	TEST_ASSERT(active_charge_limit == 550);
@@ -485,10 +480,10 @@ static int test_dual_role(void)
 	 * Insert a dual-role charger into P1 and set the override. Verify
 	 * that the override correctly changes.
 	 */
-	set_charger_role(1, DUAL_ROLE_CHARGER);
+	set_charger_role(1, CAP_DUALROLE);
 	charge_manager_set_override(1);
 	charge.current = 500;
-	charge_manager_update(CHARGE_SUPPLIER_TEST6, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST6, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 500);
@@ -498,7 +493,7 @@ static int test_dual_role(void)
 	/* Set override back to P0 and verify switch */
 	charge_manager_set_override(0);
 	charge.current = 600;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);
 	TEST_ASSERT(active_charge_limit == 600);
@@ -507,11 +502,11 @@ static int test_dual_role(void)
 
 	/* Insert a dedicated charger and verify override is removed */
 	charge.current = 0;
-	charge_manager_update(CHARGE_SUPPLIER_TEST6, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST6, 1, &charge);
 	wait_for_charge_manager_refresh();
-	set_charger_role(1, DEDICATED_CHARGER);
+	set_charger_role(1, CAP_DEDICATED);
 	charge.current = 400;
-	charge_manager_update(CHARGE_SUPPLIER_TEST6, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST6, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 400);
@@ -521,11 +516,11 @@ static int test_dual_role(void)
 	 * Verify the port is handled normally if the dual-role source is
 	 * unplugged and replaced with a dedicated source.
 	 */
-	set_charger_role(0, DEDICATED_CHARGER);
+	set_charger_role(0, CAP_DEDICATED);
 	charge.current = 0;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
 	charge.current = 500;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);
 	TEST_ASSERT(active_charge_limit == 500);
@@ -534,13 +529,13 @@ static int test_dual_role(void)
 	 * Verify that we charge from the dedicated port if a dual-role
 	 * source is also attached.
 	 */
-	set_charger_role(0, DUAL_ROLE_CHARGER);
+	set_charger_role(0, CAP_DUALROLE);
 	charge.current = 0;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
 	charge.current = 500;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
 	charge.current = 200;
-	charge_manager_update(CHARGE_SUPPLIER_TEST6, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST6, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 200);
@@ -560,7 +555,7 @@ static int test_rejected_port(void)
 	/* Set a charge on P0. */
 	charge.current = 500;
 	charge.voltage = 5000;
-	charge_manager_update(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);
 	TEST_ASSERT(active_charge_limit == 500);
@@ -568,14 +563,14 @@ static int test_rejected_port(void)
 	/* Set P0 as rejected, and verify that it doesn't become active. */
 	set_charge_port_to_reject(1);
 	charge.current = 1000;
-	charge_manager_update(CHARGE_SUPPLIER_TEST1, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST1, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);
 	TEST_ASSERT(active_charge_limit == 500);
 
 	/* Don't reject P0, and verify it can become active. */
 	set_charge_port_to_reject(CHARGE_PORT_NONE);
-	charge_manager_update(CHARGE_SUPPLIER_TEST1, 1, &charge);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST1, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 1000);
@@ -583,6 +578,93 @@ static int test_rejected_port(void)
 	return EC_SUCCESS;
 }
 
+static int test_unknown_dualrole_capability(void)
+{
+	struct charge_port_info charge;
+
+	/* Initialize table to no charge. */
+	initialize_charge_table(0, 5000, 1000);
+	TEST_ASSERT(active_charge_port == CHARGE_PORT_NONE);
+
+	/*
+	 * Set a charge on P0 with unknown dualrole capability,
+	 * verify that we don't charge from the port.
+	 */
+	charge.current = 500;
+	charge.voltage = 5000;
+	set_charger_role(0, CAP_UNKNOWN);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_dualrole(0);
+	wait_for_charge_manager_refresh();
+	TEST_ASSERT(active_charge_port == CHARGE_PORT_NONE);
+
+	/* Toggle to dedicated and verify port becomes active. */
+	set_charger_role(0, CAP_DEDICATED);
+	charge_manager_update_dualrole(0);
+	wait_for_charge_manager_refresh();
+	TEST_ASSERT(active_charge_port == 0);
+
+	/* Add dualrole charger in port 1 */
+	charge.current = 1000;
+	set_charger_role(1, CAP_DUALROLE);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 1, &charge);
+	charge_manager_update_dualrole(1);
+	wait_for_charge_manager_refresh();
+	TEST_ASSERT(active_charge_port == 0);
+
+	/* Remove charger on port 0 */
+	charge.current = 0;
+	set_charger_role(0, CAP_UNKNOWN);
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	charge_manager_update_dualrole(0);
+	wait_for_charge_manager_refresh();
+	TEST_ASSERT(active_charge_port == CHARGE_PORT_NONE);
+
+	/* Set override to charge on port 1 */
+	charge_manager_set_override(1);
+	wait_for_charge_manager_refresh();
+	TEST_ASSERT(active_charge_port == 1);
+
+	/*
+	 * Toggle port 0 to dedicated, verify that override is still kept
+	 * because there's no charge on the port.
+	 */
+	set_charger_role(0, CAP_DEDICATED);
+	charge_manager_update_dualrole(0);
+	wait_for_charge_manager_refresh();
+	TEST_ASSERT(active_charge_port == 1);
+
+	/*
+	 * Insert UNKNOWN capability charger on port 0, verify that override
+	 * is still kept.
+	 */
+	set_charger_role(0, CAP_UNKNOWN);
+	charge.current = 2000;
+	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	wait_for_charge_manager_refresh();
+	charge_manager_update_dualrole(0);
+	wait_for_charge_manager_refresh();
+	TEST_ASSERT(active_charge_port == 1);
+
+	/* Toggle to dualrole, verify that override is still kept. */
+	set_charger_role(0, CAP_DUALROLE);
+	charge_manager_update_dualrole(0);
+	wait_for_charge_manager_refresh();
+	TEST_ASSERT(active_charge_port == 1);
+
+	/* Toggle to dedicated, verify that override is removed. */
+	set_charger_role(0, CAP_UNKNOWN);
+	charge_manager_update_dualrole(0);
+	wait_for_charge_manager_refresh();
+	TEST_ASSERT(active_charge_port == 1);
+	set_charger_role(0, CAP_DEDICATED);
+	charge_manager_update_dualrole(0);
+	wait_for_charge_manager_refresh();
+	TEST_ASSERT(active_charge_port == 0);
+
+	return EC_SUCCESS;
+}
+
 void run_test(void)
 {
 	test_reset();
@@ -594,6 +676,7 @@ void run_test(void)
 	RUN_TEST(test_override);
 	RUN_TEST(test_dual_role);
 	RUN_TEST(test_rejected_port);
+	RUN_TEST(test_unknown_dualrole_capability);
 
 	test_print_result();
 }