From 96895743b0287cee5411bfc555d04123af86307d Mon Sep 17 00:00:00 2001 From: Shawn Nematbakhsh Date: Fri, 6 Feb 2015 11:16:42 -0800 Subject: 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 Reviewed-on: https://chromium-review.googlesource.com/247130 Reviewed-by: Alec Berg --- board/ryu/board.c | 45 ++++++---- board/ryu/usb_pd_policy.c | 4 +- board/samus_pd/board.c | 58 +++++++----- board/samus_pd/usb_pd_policy.c | 4 +- common/charge_manager.c | 153 ++++++++++++++++++++++---------- common/usb_pd_protocol.c | 44 +++++++++- include/charge_manager.h | 9 +- include/usb_pd.h | 9 +- test/charge_manager.c | 195 +++++++++++++++++++++++++++++------------ 9 files changed, 369 insertions(+), 152 deletions(-) diff --git a/board/ryu/board.c b/board/ryu/board.c index baf633fa96..57e2d2e2ed 100644 --- a/board/ryu/board.c +++ b/board/ryu/board.c @@ -110,19 +110,29 @@ void usb_charger_task(void) if (device_type || PI3USB9281_CHG_STATUS_ANY(charger_status)) { charge.current = pi3usb9281_get_ilim(device_type, charger_status); - charge_manager_update(type, 0, &charge); + charge_manager_update_charge(type, 0, &charge); } else { /* Detachment: update available charge to 0 */ charge.current = 0; - charge_manager_update(CHARGE_SUPPLIER_PROPRIETARY, 0, - &charge); - charge_manager_update(CHARGE_SUPPLIER_BC12_CDP, 0, - &charge); - charge_manager_update(CHARGE_SUPPLIER_BC12_DCP, 0, - &charge); - charge_manager_update(CHARGE_SUPPLIER_BC12_SDP, 0, - &charge); - charge_manager_update(CHARGE_SUPPLIER_OTHER, 0, - &charge); + charge_manager_update_charge( + CHARGE_SUPPLIER_PROPRIETARY, + 0, + &charge); + charge_manager_update_charge( + CHARGE_SUPPLIER_BC12_CDP, + 0, + &charge); + charge_manager_update_charge( + CHARGE_SUPPLIER_BC12_DCP, + 0, + &charge); + charge_manager_update_charge( + CHARGE_SUPPLIER_BC12_SDP, + 0, + &charge); + charge_manager_update_charge( + CHARGE_SUPPLIER_OTHER, + 0, + &charge); } /* notify host of power info change */ @@ -158,12 +168,13 @@ static void board_init(void) /* Initialize all pericom charge suppliers to 0 */ charge.voltage = USB_BC12_CHARGE_VOLTAGE; charge.current = 0; - charge_manager_update(CHARGE_SUPPLIER_PROPRIETARY, 0, - &charge); - charge_manager_update(CHARGE_SUPPLIER_BC12_CDP, 0, &charge); - charge_manager_update(CHARGE_SUPPLIER_BC12_DCP, 0, &charge); - charge_manager_update(CHARGE_SUPPLIER_BC12_SDP, 0, &charge); - charge_manager_update(CHARGE_SUPPLIER_OTHER, 0, &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_PROPRIETARY, + 0, + &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_BC12_CDP, 0, &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_BC12_DCP, 0, &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_BC12_SDP, 0, &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_OTHER, 0, &charge); /* Enable pericom BC1.2 interrupts. */ gpio_enable_interrupt(GPIO_USBC_BC12_INT_L); diff --git a/board/ryu/usb_pd_policy.c b/board/ryu/usb_pd_policy.c index 7556c86ba6..596e8c003b 100644 --- a/board/ryu/usb_pd_policy.c +++ b/board/ryu/usb_pd_policy.c @@ -38,7 +38,7 @@ void pd_set_input_current_limit(int port, uint32_t max_ma, struct charge_port_info charge; charge.current = max_ma; charge.voltage = supply_voltage; - charge_manager_update(CHARGE_SUPPLIER_PD, port, &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_PD, port, &charge); } void typec_set_input_current_limit(int port, uint32_t max_ma, @@ -47,7 +47,7 @@ void typec_set_input_current_limit(int port, uint32_t max_ma, struct charge_port_info charge; charge.current = max_ma; charge.voltage = supply_voltage; - charge_manager_update(CHARGE_SUPPLIER_TYPEC, port, &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_TYPEC, port, &charge); } int pd_is_valid_input_voltage(int mv) diff --git a/board/samus_pd/board.c b/board/samus_pd/board.c index 3fd3f0e219..63b435bc33 100644 --- a/board/samus_pd/board.c +++ b/board/samus_pd/board.c @@ -214,24 +214,29 @@ void usb_charger_task(void) if (device_type || PI3USB9281_CHG_STATUS_ANY(charger_status)) { charge.current = pi3usb9281_get_ilim(device_type, charger_status); - charge_manager_update(type, port, &charge); + charge_manager_update_charge(type, port, &charge); } else { /* Detachment: update available charge to 0 */ charge.current = 0; - charge_manager_update(CHARGE_SUPPLIER_PROPRIETARY, - port, - &charge); - charge_manager_update(CHARGE_SUPPLIER_BC12_CDP, - port, - &charge); - charge_manager_update(CHARGE_SUPPLIER_BC12_DCP, - port, - &charge); - charge_manager_update(CHARGE_SUPPLIER_BC12_SDP, - port, - &charge); - charge_manager_update(CHARGE_SUPPLIER_OTHER, - port, - &charge); + charge_manager_update_charge( + CHARGE_SUPPLIER_PROPRIETARY, + port, + &charge); + charge_manager_update_charge( + CHARGE_SUPPLIER_BC12_CDP, + port, + &charge); + charge_manager_update_charge( + CHARGE_SUPPLIER_BC12_DCP, + port, + &charge); + charge_manager_update_charge( + CHARGE_SUPPLIER_BC12_SDP, + port, + &charge); + charge_manager_update_charge( + CHARGE_SUPPLIER_OTHER, + port, + &charge); } /* notify host of power info change */ @@ -371,12 +376,21 @@ static void board_init(void) charge.voltage = USB_BC12_CHARGE_VOLTAGE; charge.current = 0; for (i = 0; i < PD_PORT_COUNT; i++) { - charge_manager_update(CHARGE_SUPPLIER_PROPRIETARY, i, - &charge); - charge_manager_update(CHARGE_SUPPLIER_BC12_CDP, i, &charge); - charge_manager_update(CHARGE_SUPPLIER_BC12_DCP, i, &charge); - charge_manager_update(CHARGE_SUPPLIER_BC12_SDP, i, &charge); - charge_manager_update(CHARGE_SUPPLIER_OTHER, i, &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_PROPRIETARY, + i, + &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_BC12_CDP, + i, + &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_BC12_DCP, + i, + &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_BC12_SDP, + i, + &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_OTHER, + i, + &charge); } /* Enable pericom BC1.2 interrupts. */ diff --git a/board/samus_pd/usb_pd_policy.c b/board/samus_pd/usb_pd_policy.c index 6a5a0043c0..7d638dfd0c 100644 --- a/board/samus_pd/usb_pd_policy.c +++ b/board/samus_pd/usb_pd_policy.c @@ -111,7 +111,7 @@ void pd_set_input_current_limit(int port, uint32_t max_ma, struct charge_port_info charge; charge.current = max_ma; charge.voltage = supply_voltage; - charge_manager_update(CHARGE_SUPPLIER_PD, port, &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_PD, port, &charge); /* notify host of power info change */ pd_send_host_event(PD_EVENT_POWER_CHANGE); @@ -123,7 +123,7 @@ void typec_set_input_current_limit(int port, uint32_t max_ma, struct charge_port_info charge; charge.current = max_ma; charge.voltage = supply_voltage; - charge_manager_update(CHARGE_SUPPLIER_TYPEC, port, &charge); + charge_manager_update_charge(CHARGE_SUPPLIER_TYPEC, port, &charge); /* notify host of power info change */ pd_send_host_event(PD_EVENT_POWER_CHANGE); diff --git a/common/charge_manager.c b/common/charge_manager.c index d11fef91d8..c3e5b4d0ce 100644 --- a/common/charge_manager.c +++ b/common/charge_manager.c @@ -43,6 +43,11 @@ static int override_port = OVERRIDE_OFF; static int delayed_override_port = OVERRIDE_OFF; static timestamp_t delayed_override_deadline; +enum charge_manager_change_type { + CHANGE_CHARGE, + CHANGE_DUALROLE, +}; + /** * Initialize available charge. Run before board init, so board init can * initialize data, if needed. @@ -127,7 +132,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); + r->dualrole = (pd_get_partner_dualrole_capable(port) == CAP_DUALROLE); if (sup == CHARGE_SUPPLIER_NONE || r->role == USB_PD_PORT_POWER_SOURCE) { @@ -226,7 +231,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) && + if (pd_get_partner_dualrole_capable(port) == CAP_DUALROLE && pd_get_role(port) == PD_ROLE_SINK) pd_request_power_swap(port); } @@ -277,7 +282,8 @@ 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) && + if (pd_get_partner_dualrole_capable(j) != + CAP_DEDICATED && override_port != j) continue; @@ -425,60 +431,117 @@ static void charge_manager_refresh(void) } DECLARE_DEFERRED(charge_manager_refresh); -/** - * Update available charge for a given port / supplier. - * - * @param supplier Charge supplier to update. - * @param port Charge port to update. - * @param charge Charge port current / voltage. - */ -void charge_manager_update(int supplier, - int port, - struct charge_port_info *charge) +static void charge_manager_make_change(enum charge_manager_change_type change, + int supplier, + int port, + struct charge_port_info *charge) { - ASSERT(supplier >= 0 && supplier < CHARGE_SUPPLIER_COUNT); - ASSERT(port >= 0 && port < PD_PORT_COUNT); - - /* Update charge table if needed. */ - if (available_charge[supplier][port].current != charge->current || - available_charge[supplier][port].voltage != charge->voltage) { - /* Remove override when a dedicated charger is plugged */ - 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, - -1); + int i; + int clear_override = 0; + + /* Determine if this is a change which can affect charge status */ + switch (change) { + case CHANGE_CHARGE: + /* Ignore changes where charge is identical */ + if (available_charge[supplier][port].current == + charge->current && + available_charge[supplier][port].voltage == + charge->voltage) + return; + if (charge->current > 0 && + available_charge[supplier][port].current == 0) + clear_override = 1; + break; + case CHANGE_DUALROLE: + /* + * 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) + return; + /* Clear override only if a charge is present on the port */ + for (i = 0; i < CHARGE_SUPPLIER_COUNT; ++i) + if (available_charge[i][port].current > 0) { + clear_override = 1; + break; } + /* + * If there is no charge present on the port, the dualrole + * change is meaningless to charge_manager. + */ + if (!clear_override) + return; + break; + } + + /* Remove override when a dedicated charger is plugged */ + if (clear_override && override_port != port && + pd_get_partner_dualrole_capable(port) == CAP_DEDICATED) { + 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, + -1); } + } + + if (change == CHANGE_CHARGE) { available_charge[supplier][port].current = charge->current; available_charge[supplier][port].voltage = charge->voltage; /* * If we have a charge on our delayed override port within * the deadline, make it our override port. - */ - if (port == delayed_override_port && - charge->current > 0 && + */ + if (port == delayed_override_port && charge->current > 0 && pd_get_role(delayed_override_port) == PD_ROLE_SINK && get_time().val < delayed_override_deadline.val) charge_manager_set_override(port); - - /* - * Don't call charge_manager_refresh unless all ports + - * suppliers have reported in. We don't want to make changes - * to our charge port until we are certain we know what is - * attached. - */ - if (charge_manager_is_seeded()) - hook_call_deferred(charge_manager_refresh, 0); } + + /* + * Don't call charge_manager_refresh unless all ports + + * suppliers have reported in. We don't want to make changes + * to our charge port until we are certain we know what is + * attached. + */ + if (charge_manager_is_seeded()) + hook_call_deferred(charge_manager_refresh, 0); +} + +/** + * Update available charge for a given port / supplier. + * + * @param supplier Charge supplier to update. + * @param port Charge port to update. + * @param charge Charge port current / voltage. + */ +void charge_manager_update_charge(int supplier, + int port, + struct charge_port_info *charge) +{ + ASSERT(supplier >= 0 && supplier < CHARGE_SUPPLIER_COUNT); + ASSERT(port >= 0 && port < PD_PORT_COUNT); + ASSERT(charge != NULL); + + charge_manager_make_change(CHANGE_CHARGE, supplier, port, charge); +} + +/** + * 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(). + * + * @param port Charge port which changed. + */ +void charge_manager_update_dualrole(int port) +{ + ASSERT(port >= 0 && port < PD_PORT_COUNT); + charge_manager_make_change(CHANGE_DUALROLE, 0, port, NULL); } /** @@ -540,7 +603,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)) { + pd_get_partner_dualrole_capable(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 049e53f13f..77329ef5a4 100644 --- a/common/usb_pd_protocol.c +++ b/common/usb_pd_protocol.c @@ -280,6 +280,11 @@ 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. */ @@ -399,6 +404,10 @@ static inline void set_state(int port, enum pd_states next_state) #endif 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); +#endif #ifdef CONFIG_USB_PD_ALT_MODE_DFP pd_dfp_exit_mode(port, 0, 0); #endif @@ -968,10 +977,19 @@ static void pd_update_pdo_flags(int port, uint32_t pdo) { /* can only parse PDO flags if type is fixed */ if ((pdo & PDO_TYPE_MASK) == PDO_TYPE_FIXED) { - if (pdo & PDO_FIXED_DUAL_ROLE) + if (pdo & PDO_FIXED_DUAL_ROLE) { pd[port].flags |= PD_FLAGS_PARTNER_DR_POWER; - else +#ifdef CONFIG_CHARGE_MANAGER + pd[port].dualrole_capability = CAP_DUALROLE; + charge_manager_update_dualrole(port); +#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); +#endif + } if (pdo & PDO_FIXED_DATA_SWAP) pd[port].flags |= PD_FLAGS_PARTNER_DR_DATA; @@ -1655,11 +1673,13 @@ int pd_get_polarity(int port) return pd[port].polarity; } -int pd_get_partner_dualrole_capable(int port) +#ifdef CONFIG_CHARGE_MANAGER +enum dualrole_capabilities pd_get_partner_dualrole_capable(int port) { /* return dualrole status of port partner */ - return pd[port].flags & PD_FLAGS_PARTNER_DR_POWER; + return pd[port].dualrole_capability; } +#endif /* CONFIG_CHARGE_MANAGER */ int pd_get_partner_data_swap_capable(int port) { @@ -1791,6 +1811,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; #ifdef CONFIG_USB_PD_DUAL_ROLE /* If sink, set initial type-C current limit based on vbus */ if (pd_snk_is_vbus_provided(port)) { @@ -2673,6 +2694,21 @@ void pd_task(void) hard_reset_count++; if (pd[port].last_state != pd[port].task_state) hard_reset_sent = 0; +#ifdef CONFIG_CHARGE_MANAGER + if (pd[port].last_state == PD_STATE_SNK_DISCOVERY) { + /* + * If discovery timed out, assume that we + * have a dedicated charger attached. This + * may not be a correct assumption according + * to the specification, but it generally + * works in practice and the harmful + * effects of a wrong assumption here + * are minimal. + */ + pd[port].dualrole_capability = CAP_DEDICATED; + charge_manager_update_dualrole(port); + } +#endif /* try sending hard reset until it succeeds */ if (!hard_reset_sent) { diff --git a/include/charge_manager.h b/include/charge_manager.h index 0b1de3186d..70afd2135e 100644 --- a/include/charge_manager.h +++ b/include/charge_manager.h @@ -22,9 +22,12 @@ struct charge_port_info { }; /* Called by charging tasks to update their available charge */ -void charge_manager_update(int supplier, - int charge_port, - struct charge_port_info *charge); +void charge_manager_update_charge(int supplier, + int port, + struct charge_port_info *charge); + +/* Called by charging tasks to indicate partner dualrole capability change */ +void charge_manager_update_dualrole(int port); /* 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 410251775f..d177007239 100644 --- a/include/usb_pd.h +++ b/include/usb_pd.h @@ -1283,12 +1283,19 @@ 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 */ -int pd_get_partner_dualrole_capable(int port); +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 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(); } -- cgit v1.2.1