charge_manager: Request power swap when switching from dual-role override port

Ports should have source roles by default, and should go back to being
sources once we stop charging from them.

BUG=chrome-os-partner:31195
TEST=Manual on Samus. Connect Samus to dual-role port. Set override
port, verify that Samus charges. Attach dedicated charger, verify that
the dual-role port becomes a charge source again. Also pass unit tests.
BRANCH=Samus

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

2 files changed, 73 insertions, 6 deletions

diff --git a/common/charge_manager.c b/common/charge_manager.c
index a74ba3a0d6..651cf3af5e 100644
--- a/common/charge_manager.c
+++ b/common/charge_manager.c
@@ -87,6 +87,21 @@ static int charge_manager_is_seeded(void)
 }
 
 /**
+ * Perform cleanup operations on an override port, when switching to a
+ * different port. This involves switching the port from sink to source,
+ * if applicable.
+ */
+static void charge_manager_cleanup_override_port(int port)
+{
+	if (port < 0 || port >= PD_PORT_COUNT)
+		return;
+
+	if (pd_get_partner_dualrole_capable(port) &&
+	    pd_get_role(port) == PD_ROLE_SINK)
+		pd_request_power_swap(port);
+}
+
+/**
  * Select the 'best' charge port, as defined by the supplier heirarchy and the
  * ability of the port to provide power.
  */
@@ -141,10 +156,6 @@ static void charge_manager_get_best_charge_port(int *new_port,
 				}
 			}
 
-		/* Clear override if no charge is available on override port */
-		if (override_port != OVERRIDE_OFF &&
-		    override_port != port)
-			override_port = OVERRIDE_OFF;
 	}
 
 	*new_port = port;
@@ -183,6 +194,16 @@ static void charge_manager_refresh(void)
 			available_charge[i][new_port].current = 0;
 	}
 
+	/*
+	 * Clear override if it wasn't selected as the 'best' port -- it means
+	 * that no charge is available on the port, or the port was rejected.
+	 */
+	if (override_port >= 0 &&
+	    override_port != new_port) {
+		charge_manager_cleanup_override_port(override_port);
+		override_port = OVERRIDE_OFF;
+	}
+
 	if (new_supplier == CHARGE_SUPPLIER_NONE) {
 		new_charge_current = 0;
 		new_charge_current_uncapped = 0;
@@ -259,8 +280,11 @@ void charge_manager_update(int supplier,
 		if (available_charge[supplier][port].current == 0 &&
 		    charge->current > 0 &&
 		    !pd_get_partner_dualrole_capable(port)) {
+			charge_manager_cleanup_override_port(override_port);
 			override_port = OVERRIDE_OFF;
 			if (delayed_override_port != OVERRIDE_OFF) {
+				charge_manager_cleanup_override_port(
+					delayed_override_port);
 				delayed_override_port = OVERRIDE_OFF;
 				hook_call_deferred(
 					board_charge_manager_override_timeout,
@@ -322,9 +346,13 @@ int charge_manager_set_override(int port)
 	int retval = EC_SUCCESS;
 
 	ASSERT(port >= OVERRIDE_DONT_CHARGE && port < PD_PORT_COUNT);
-	/* Supersede any pending delayed overrides. */
 
+	/* Supersede any pending delayed overrides. */
 	if (delayed_override_port != OVERRIDE_OFF) {
+		if (delayed_override_port != port)
+			charge_manager_cleanup_override_port(
+				delayed_override_port);
+
 		delayed_override_port = OVERRIDE_OFF;
 		hook_call_deferred(
 			board_charge_manager_override_timeout, -1);
@@ -333,6 +361,7 @@ int charge_manager_set_override(int port)
 	/* Set the override port if it's a sink. */
 	if (port < 0 || pd_get_role(port) == PD_ROLE_SINK) {
 		if (override_port != port) {
+			charge_manager_cleanup_override_port(override_port);
 			override_port = port;
 			if (charge_manager_is_seeded())
 				hook_call_deferred(charge_manager_refresh, 0);
diff --git a/test/charge_manager.c b/test/charge_manager.c
index fc184bebea..db81105ff6 100644
--- a/test/charge_manager.c
+++ b/test/charge_manager.c
@@ -399,15 +399,16 @@ static int test_override(void)
 	charge.current = 200;
 	charge_manager_update(CHARGE_SUPPLIER_TEST1, 0, &charge);
 	wait_for_charge_manager_refresh();
-	TEST_ASSERT(pd_get_role(0) == PD_ROLE_SINK);
 	TEST_ASSERT(active_charge_port == 0);
 	TEST_ASSERT(active_charge_limit == 200);
+	TEST_ASSERT(pd_get_role(0) == PD_ROLE_SINK);
 
 	/* Set override to "don't charge", then verify we're not charging */
 	charge_manager_set_override(OVERRIDE_DONT_CHARGE);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == CHARGE_PORT_NONE);
 	TEST_ASSERT(active_charge_limit == 0);
+	TEST_ASSERT(pd_get_role(0) == PD_ROLE_SOURCE);
 
 	/* Update a charge supplier, verify that we still aren't charging */
 	charge.current = 200;
@@ -415,12 +416,14 @@ static int test_override(void)
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == CHARGE_PORT_NONE);
 	TEST_ASSERT(active_charge_limit == 0);
+	TEST_ASSERT(pd_get_role(0) == PD_ROLE_SOURCE);
 
 	/* Turn override off, verify that we go back to the correct charge */
 	charge_manager_set_override(OVERRIDE_OFF);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 500);
+	TEST_ASSERT(pd_get_role(0) == PD_ROLE_SOURCE);
 
 	return EC_SUCCESS;
 }
@@ -449,25 +452,59 @@ static int test_dual_role(void)
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);
 	TEST_ASSERT(active_charge_limit == 500);
+	TEST_ASSERT(pd_get_role(0) == PD_ROLE_SINK);
 
 	/* Remove override and verify we go back to not charging */
 	charge_manager_set_override(OVERRIDE_OFF);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == CHARGE_PORT_NONE);
 	TEST_ASSERT(active_charge_limit == 0);
+	TEST_ASSERT(pd_get_role(0) == PD_ROLE_SOURCE);
 
 	/* 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);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);
+	TEST_ASSERT(active_charge_limit == 550);
+	TEST_ASSERT(pd_get_role(0) == PD_ROLE_SINK);
+
+	/*
+	 * Insert a dual-role charger into P1 and set the override. Verify
+	 * that the override correctly changes.
+	 */
+	set_charger_role(1, DUAL_ROLE_CHARGER);
+	charge_manager_set_override(1);
+	charge.current = 500;
+	charge_manager_update(CHARGE_SUPPLIER_TEST6, 1, &charge);
+	wait_for_charge_manager_refresh();
+	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 500);
+	TEST_ASSERT(pd_get_role(1) == PD_ROLE_SINK);
+	TEST_ASSERT(pd_get_role(0) == PD_ROLE_SOURCE);
+
+	/* Set override back to P0 and verify switch */
+	charge_manager_set_override(0);
+	charge.current = 600;
+	charge_manager_update(CHARGE_SUPPLIER_TEST2, 0, &charge);
+	wait_for_charge_manager_refresh();
+	TEST_ASSERT(active_charge_port == 0);
+	TEST_ASSERT(active_charge_limit == 600);
+	TEST_ASSERT(pd_get_role(0) == PD_ROLE_SINK);
+	TEST_ASSERT(pd_get_role(1) == PD_ROLE_SOURCE);
 
 	/* Insert a dedicated charger and verify override is removed */
+	charge.current = 0;
+	charge_manager_update(CHARGE_SUPPLIER_TEST6, 1, &charge);
+	wait_for_charge_manager_refresh();
+	set_charger_role(1, DEDICATED_CHARGER);
 	charge.current = 400;
 	charge_manager_update(CHARGE_SUPPLIER_TEST6, 1, &charge);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 400);
+	TEST_ASSERT(pd_get_role(0) == PD_ROLE_SOURCE);
 
 	/*
 	 * Verify the port is handled normally if the dual-role source is
@@ -496,6 +533,7 @@ static int test_dual_role(void)
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 	TEST_ASSERT(active_charge_limit == 200);
+	TEST_ASSERT(pd_get_role(0) == PD_ROLE_SOURCE);
 
 	return EC_SUCCESS;
 }