charge_manager: Store dualrole capability in charge manager

Since charge manager is now informed of all capability changes as they
happen, it makes sense to store the port capability within charge manager,
rather than storing in pd.

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: I3669050b37ddd67f6608bf790a07e74f86b6ac01
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/247724
Reviewed-by: Alec Berg <alecaberg@chromium.org>

5 files changed, 51 insertions, 90 deletions

diff --git a/common/charge_manager.c b/common/charge_manager.c
index c3e5b4d0ce..7ee86081a6 100644
--- a/common/charge_manager.c
+++ b/common/charge_manager.c
@@ -32,6 +32,9 @@ static struct charge_port_info available_charge[CHARGE_SUPPLIER_COUNT]
  */
 static int charge_ceil[PD_PORT_COUNT];
 
+/* Dual-role capability of attached partner port */
+static enum dualrole_capabilities dualrole_capability[PD_PORT_COUNT];
+
 /* Store current state of port enable / charge current. */
 static int charge_port = CHARGE_PORT_NONE;
 static int charge_current = CHARGE_CURRENT_UNINITIALIZED;
@@ -64,6 +67,7 @@ static void charge_manager_init(void)
 				CHARGE_VOLTAGE_UNINITIALIZED;
 		}
 		charge_ceil[i] = CHARGE_CEIL_NONE;
+		dualrole_capability[i] = CAP_UNKNOWN;
 	}
 }
 DECLARE_HOOK(HOOK_INIT, charge_manager_init, HOOK_PRIO_DEFAULT-1);
@@ -132,7 +136,7 @@ static void charge_manager_fill_power_info(int port,
 		r->role = USB_PD_PORT_POWER_DISCONNECTED;
 
 	/* Is port partner dual-role capable */
-	r->dualrole = (pd_get_partner_dualrole_capable(port) == CAP_DUALROLE);
+	r->dualrole = (dualrole_capability[port] == CAP_DUALROLE);
 
 	if (sup == CHARGE_SUPPLIER_NONE ||
 	    r->role == USB_PD_PORT_POWER_SOURCE) {
@@ -231,7 +235,7 @@ static void charge_manager_cleanup_override_port(int port)
 	if (port < 0 || port >= PD_PORT_COUNT)
 		return;
 
-	if (pd_get_partner_dualrole_capable(port) == CAP_DUALROLE &&
+	if (dualrole_capability[port] == CAP_DUALROLE &&
 	    pd_get_role(port) == PD_ROLE_SINK)
 		pd_request_power_swap(port);
 }
@@ -282,8 +286,7 @@ static void charge_manager_get_best_charge_port(int *new_port,
 				 * Don't charge from a dual-role port unless
 				 * it is our override port.
 				 */
-				if (pd_get_partner_dualrole_capable(j) !=
-				    CAP_DEDICATED &&
+				if (dualrole_capability[j] != CAP_DEDICATED &&
 				    override_port != j)
 					continue;
 
@@ -457,7 +460,7 @@ static void charge_manager_make_change(enum charge_manager_change_type change,
 		 * Ignore all except for transition to non-dualrole,
 		 * which may occur some time after we see a charge
 		 */
-		if (pd_get_partner_dualrole_capable(port) != CAP_DEDICATED)
+		if (dualrole_capability[port] != CAP_DEDICATED)
 			return;
 		/* Clear override only if a charge is present on the port */
 		for (i = 0; i < CHARGE_SUPPLIER_COUNT; ++i)
@@ -476,7 +479,7 @@ static void charge_manager_make_change(enum charge_manager_change_type change,
 
 	/* Remove override when a dedicated charger is plugged */
 	if (clear_override && override_port != port &&
-	    pd_get_partner_dualrole_capable(port) == CAP_DEDICATED) {
+	    dualrole_capability[port] == CAP_DEDICATED) {
 		charge_manager_cleanup_override_port(override_port);
 		override_port = OVERRIDE_OFF;
 		if (delayed_override_port != OVERRIDE_OFF) {
@@ -532,16 +535,19 @@ void charge_manager_update_charge(int supplier,
 }
 
 /**
- * Notify charge_manager of a partner dualrole capability change. There is
- * no capability parameter to this function, since the capability can be
- * checked with pd_get_partner_dualrole_capable().
+ * Notify charge_manager of a partner dualrole capability change.
  *
  * @param port			Charge port which changed.
+ * @param cap			New port capability.
  */
-void charge_manager_update_dualrole(int port)
+void charge_manager_update_dualrole(int port, enum dualrole_capabilities cap)
 {
 	ASSERT(port >= 0 && port < PD_PORT_COUNT);
-	charge_manager_make_change(CHANGE_DUALROLE, 0, port, NULL);
+	/* Ignore when capability is unchanged */
+	if (cap != dualrole_capability[port]) {
+		dualrole_capability[port] = cap;
+		charge_manager_make_change(CHANGE_DUALROLE, 0, port, NULL);
+	}
 }
 
 /**
@@ -603,7 +609,7 @@ int charge_manager_set_override(int port)
 	 * power swap and set the delayed override for swap completion.
 	 */
 	else if (pd_get_role(port) != PD_ROLE_SINK &&
-		 pd_get_partner_dualrole_capable(port) == CAP_DUALROLE) {
+		 dualrole_capability[port] == CAP_DUALROLE) {
 		delayed_override_deadline.val = get_time().val +
 						POWER_SWAP_TIMEOUT;
 		delayed_override_port = port;
diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c
index c8004d3daf..18a217528b 100644
--- a/common/usb_pd_protocol.c
+++ b/common/usb_pd_protocol.c
@@ -281,11 +281,6 @@ static struct pd_protocol {
 	int prev_request_mv;
 #endif
 
-#ifdef CONFIG_CHARGE_MANAGER
-	/* Dual-role capability of attached partner port */
-	enum dualrole_capabilities dualrole_capability;
-#endif
-
 	/* PD state for Vendor Defined Messages */
 	enum vdm_states vdm_state;
 	/* Timeout for the current vdm state.  Set to 0 for no timeout. */
@@ -406,8 +401,7 @@ static inline void set_state(int port, enum pd_states next_state)
 		pd[port].dev_id = 0;
 		pd[port].flags &= ~PD_FLAGS_RESET_ON_DISCONNECT_MASK;
 #ifdef CONFIG_CHARGE_MANAGER
-		pd[port].dualrole_capability = CAP_UNKNOWN;
-		charge_manager_update_dualrole(port);
+		charge_manager_update_dualrole(port, CAP_UNKNOWN);
 #endif
 #ifdef CONFIG_USB_PD_ALT_MODE_DFP
 		pd_dfp_exit_mode(port, 0, 0);
@@ -981,14 +975,12 @@ static void pd_update_pdo_flags(int port, uint32_t pdo)
 		if (pdo & PDO_FIXED_DUAL_ROLE) {
 			pd[port].flags |= PD_FLAGS_PARTNER_DR_POWER;
 #ifdef CONFIG_CHARGE_MANAGER
-			pd[port].dualrole_capability = CAP_DUALROLE;
-			charge_manager_update_dualrole(port);
+			charge_manager_update_dualrole(port, CAP_DUALROLE);
 #endif
 		} else {
 			pd[port].flags &= ~PD_FLAGS_PARTNER_DR_POWER;
 #ifdef CONFIG_CHARGE_MANAGER
-			pd[port].dualrole_capability = CAP_DEDICATED;
-			charge_manager_update_dualrole(port);
+			charge_manager_update_dualrole(port, CAP_DEDICATED);
 #endif
 		}
 
@@ -1674,14 +1666,6 @@ int pd_get_polarity(int port)
 	return pd[port].polarity;
 }
 
-#ifdef CONFIG_CHARGE_MANAGER
-enum dualrole_capabilities pd_get_partner_dualrole_capable(int port)
-{
-	/* return dualrole status of port partner */
-	return pd[port].dualrole_capability;
-}
-#endif /* CONFIG_CHARGE_MANAGER */
-
 int pd_get_partner_data_swap_capable(int port)
 {
 	/* return data swap capable status of port partner */
@@ -1812,7 +1796,7 @@ void pd_task(void)
 #ifdef CONFIG_CHARGE_MANAGER
 	/* Initialize PD supplier current limit to 0 */
 	pd_set_input_current_limit(port, 0, 0);
-	pd[port].dualrole_capability = CAP_UNKNOWN;
+	charge_manager_update_dualrole(port, CAP_UNKNOWN);
 #ifdef CONFIG_USB_PD_DUAL_ROLE
 	/* If sink, set initial type-C current limit based on vbus */
 	if (pd_snk_is_vbus_provided(port)) {
@@ -2719,8 +2703,8 @@ void pd_task(void)
 				 * effects of a wrong assumption here
 				 * are minimal.
 				 */
-				pd[port].dualrole_capability = CAP_DEDICATED;
-				charge_manager_update_dualrole(port);
+				charge_manager_update_dualrole(port,
+							       CAP_DEDICATED);
 			}
 #endif
 
diff --git a/include/charge_manager.h b/include/charge_manager.h
index 70afd2135e..0b5c4f7cb4 100644
--- a/include/charge_manager.h
+++ b/include/charge_manager.h
@@ -26,8 +26,15 @@ void charge_manager_update_charge(int supplier,
 				  int port,
 				  struct charge_port_info *charge);
 
+/* Partner port dualrole capabilities */
+enum dualrole_capabilities {
+	CAP_UNKNOWN,
+	CAP_DUALROLE,
+	CAP_DEDICATED,
+};
+
 /* Called by charging tasks to indicate partner dualrole capability change */
-void charge_manager_update_dualrole(int port);
+void charge_manager_update_dualrole(int port, enum dualrole_capabilities cap);
 
 /* Update charge ceiling for a given port */
 void charge_manager_set_ceil(int port, int ceil);
diff --git a/include/usb_pd.h b/include/usb_pd.h
index d177007239..2e4e62f70e 100644
--- a/include/usb_pd.h
+++ b/include/usb_pd.h
@@ -1283,20 +1283,6 @@ int pd_is_connected(int port);
  */
 int pd_get_polarity(int port);
 
-/* Partner port dualrole capabilities */
-enum dualrole_capabilities {
-	CAP_UNKNOWN,
-	CAP_DUALROLE,
-	CAP_DEDICATED,
-};
-
-/**
- * Get port partner dual-role capable status
- *
- * @param port USB-C port number
- */
-enum dualrole_capabilities pd_get_partner_dualrole_capable(int port);
-
 /**
  * Get port partner data swap capable status
  *
diff --git a/test/charge_manager.c b/test/charge_manager.c
index 977f516496..d504d9daf5 100644
--- a/test/charge_manager.c
+++ b/test/charge_manager.c
@@ -34,7 +34,6 @@ 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] = { CAP_DEDICATED, CAP_DEDICATED };
 static int power_role[PD_PORT_COUNT];
 
 /* Callback functions called by CM on state change */
@@ -75,21 +74,6 @@ static void clear_new_power_requests(void)
 		new_power_request[i] = 0;
 }
 
-/*
- * Set dual-role capability attribute of port. Note that this capability
- * 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, enum dualrole_capabilities cap)
-{
-	dual_role_capable[port] = cap;
-}
-
-enum dualrole_capabilities pd_get_partner_dualrole_capable(int port)
-{
-	return dual_role_capable[port];
-}
-
 static void pd_set_role(int port, int role)
 {
 	power_role[port] = role;
@@ -125,7 +109,7 @@ 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, CAP_DEDICATED);
+		charge_manager_update_dualrole(i, CAP_DEDICATED);
 		pd_set_role(i, PD_ROLE_SINK);
 		for (j = 0; j < CHARGE_SUPPLIER_COUNT; ++j)
 			charge_manager_update_charge(j, i, &charge);
@@ -149,6 +133,9 @@ static int test_initialization(void)
 	/* Initialize all supplier/port pairs, except for the last one */
 	for (i = 0; i < CHARGE_SUPPLIER_COUNT; ++i)
 		for (j = 0; j < PD_PORT_COUNT; ++j) {
+			if (i == 0)
+				charge_manager_update_dualrole(j,
+							       CAP_DEDICATED);
 			if (i == CHARGE_SUPPLIER_COUNT - 1 &&
 			    j == PD_PORT_COUNT - 1)
 				break;
@@ -398,7 +385,7 @@ static int test_override(void)
 	 * is successful.
 	 */
 	charge_manager_set_override(OVERRIDE_DONT_CHARGE);
-	set_charger_role(0, CAP_DUALROLE);
+	charge_manager_update_dualrole(0, CAP_DUALROLE);
 	pd_set_role(0, PD_ROLE_SOURCE);
 	charge_manager_set_override(0);
 	wait_for_charge_manager_refresh();
@@ -445,7 +432,7 @@ 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, CAP_DUALROLE);
+	charge_manager_update_dualrole(0, CAP_DUALROLE);
 	charge.current = 500;
 	charge.voltage = 5000;
 	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
@@ -480,7 +467,7 @@ 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, CAP_DUALROLE);
+	charge_manager_update_dualrole(1, CAP_DUALROLE);
 	charge_manager_set_override(1);
 	charge.current = 500;
 	charge_manager_update_charge(CHARGE_SUPPLIER_TEST6, 1, &charge);
@@ -504,7 +491,7 @@ static int test_dual_role(void)
 	charge.current = 0;
 	charge_manager_update_charge(CHARGE_SUPPLIER_TEST6, 1, &charge);
 	wait_for_charge_manager_refresh();
-	set_charger_role(1, CAP_DEDICATED);
+	charge_manager_update_dualrole(1, CAP_DEDICATED);
 	charge.current = 400;
 	charge_manager_update_charge(CHARGE_SUPPLIER_TEST6, 1, &charge);
 	wait_for_charge_manager_refresh();
@@ -516,7 +503,7 @@ 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, CAP_DEDICATED);
+	charge_manager_update_dualrole(0, CAP_DEDICATED);
 	charge.current = 0;
 	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
 	charge.current = 500;
@@ -529,7 +516,7 @@ 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, CAP_DUALROLE);
+	charge_manager_update_dualrole(0, CAP_DUALROLE);
 	charge.current = 0;
 	charge_manager_update_charge(CHARGE_SUPPLIER_TEST2, 0, &charge);
 	charge.current = 500;
@@ -592,31 +579,27 @@ static int test_unknown_dualrole_capability(void)
 	 */
 	charge.current = 500;
 	charge.voltage = 5000;
-	set_charger_role(0, CAP_UNKNOWN);
+	charge_manager_update_dualrole(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);
+	charge_manager_update_dualrole(0, CAP_DEDICATED);
 	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_dualrole(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_dualrole(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);
 
@@ -629,8 +612,7 @@ static int test_unknown_dualrole_capability(void)
 	 * 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);
+	charge_manager_update_dualrole(0, CAP_DEDICATED);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
 
@@ -638,27 +620,23 @@ static int test_unknown_dualrole_capability(void)
 	 * Insert UNKNOWN capability charger on port 0, verify that override
 	 * is still kept.
 	 */
-	set_charger_role(0, CAP_UNKNOWN);
+	charge_manager_update_dualrole(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);
+	charge_manager_update_dualrole(0, CAP_DUALROLE);
 	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);
+	charge_manager_update_dualrole(0, CAP_UNKNOWN);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 1);
-	set_charger_role(0, CAP_DEDICATED);
-	charge_manager_update_dualrole(0);
+	charge_manager_update_dualrole(0, CAP_DEDICATED);
 	wait_for_charge_manager_refresh();
 	TEST_ASSERT(active_charge_port == 0);