From 33d4fba9daf641d8f52171bc1a1de823a1afb650 Mon Sep 17 00:00:00 2001 From: Jeremy Bettis Date: Tue, 6 Apr 2021 15:02:55 -0600 Subject: ec: Filter non-FIXED PDOs in servo_v4{p1} Add a new config CONFIG_USB_PD_ONLY_FIXED_PDOS. If that config is enabled, ignore non-FIXED PDOs in both the console command `ada_srccaps` and also when selecting the preferred PDO for a voltage. Enable CONFIG_USB_PD_ONLY_FIXED_PDOS for servo_v4 and servo_v4p1, since they don't expose non-fixed PDO in their srccaps. Without this change, there is a risk that the "best" PDO for a given voltage will be non-FIXED and then that voltage just won't be supported at all. BRANCH=none BUG=b:178484932 TEST=added Change-Id: I0d1187ca372120c7fe21d627e1b82b59f6334add Signed-off-by: Jeremy Bettis Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2809353 Tested-by: Jeremy Bettis Commit-Queue: Keith Short Reviewed-by: Keith Short --- board/servo_v4/board.h | 1 + board/servo_v4/usb_pd_policy.c | 4 ++-- board/servo_v4p1/board.h | 1 + board/servo_v4p1/usb_pd_policy.c | 4 ++-- common/usb_pd_dual_role.c | 3 +++ include/config.h | 6 +++++ test/build.mk | 2 ++ test/test_config.h | 11 +++++++++ test/usb_pd_pdo_fixed.tasklist | 10 +++++++++ test/usb_pd_pdo_fixed_test.c | 47 +++++++++++++++++++++++++++++++++++++++ zephyr/Kconfig.usbc | 7 ++++++ zephyr/shim/include/config_chip.h | 5 +++++ 12 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 test/usb_pd_pdo_fixed.tasklist create mode 100644 test/usb_pd_pdo_fixed_test.c diff --git a/board/servo_v4/board.h b/board/servo_v4/board.h index 5583af223d..9a0973fe41 100644 --- a/board/servo_v4/board.h +++ b/board/servo_v4/board.h @@ -131,6 +131,7 @@ #undef CONFIG_USB_PD_PULLUP #define CONFIG_USB_PD_PULLUP TYPEC_RP_USB #define CONFIG_USB_PD_VBUS_MEASURE_NOT_PRESENT +#define CONFIG_USB_PD_ONLY_FIXED_PDOS /* Don't automatically change roles */ #undef CONFIG_USB_PD_INITIAL_DRP_STATE diff --git a/board/servo_v4/usb_pd_policy.c b/board/servo_v4/usb_pd_policy.c index e85c6b0328..f3df427851 100644 --- a/board/servo_v4/usb_pd_policy.c +++ b/board/servo_v4/usb_pd_policy.c @@ -1214,8 +1214,8 @@ static int cmd_ada_srccaps(int argc, char *argv[]) for (i = 0; i < pd_get_src_cap_cnt(CHG); ++i) { uint32_t max_ma, max_mv, unused; - /* It's an supported Augmented PDO (PD3.0) */ - if ((ada_srccaps[i] & PDO_TYPE_MASK) == PDO_TYPE_AUGMENTED) + if (IS_ENABLED(CONFIG_USB_PD_ONLY_FIXED_PDOS) && + (ada_srccaps[i] & PDO_TYPE_MASK) != PDO_TYPE_FIXED) continue; pd_extract_pdo_power(ada_srccaps[i], &max_ma, &max_mv, &unused); diff --git a/board/servo_v4p1/board.h b/board/servo_v4p1/board.h index a43ccbf9f6..b28f18a60a 100644 --- a/board/servo_v4p1/board.h +++ b/board/servo_v4p1/board.h @@ -213,6 +213,7 @@ /* Default pull-up should not be Rp3a0 due to Cr50 */ #define CONFIG_USB_PD_PULLUP TYPEC_RP_USB #define CONFIG_USB_PD_VBUS_MEASURE_NOT_PRESENT +#define CONFIG_USB_PD_ONLY_FIXED_PDOS #define CONFIG_USB_PD_ALT_MODE #define CONFIG_USBC_SS_MUX diff --git a/board/servo_v4p1/usb_pd_policy.c b/board/servo_v4p1/usb_pd_policy.c index 2a1c929193..93f4a2034c 100644 --- a/board/servo_v4p1/usb_pd_policy.c +++ b/board/servo_v4p1/usb_pd_policy.c @@ -1268,8 +1268,8 @@ static int cmd_ada_srccaps(int argc, char *argv[]) for (i = 0; i < pd_get_src_cap_cnt(CHG); ++i) { uint32_t max_ma, max_mv, unused; - /* It's an supported Augmented PDO (PD3.0) */ - if ((ada_srccaps[i] & PDO_TYPE_MASK) == PDO_TYPE_AUGMENTED) + if (IS_ENABLED(CONFIG_USB_PD_ONLY_FIXED_PDOS) && + (ada_srccaps[i] & PDO_TYPE_MASK) != PDO_TYPE_FIXED) continue; pd_extract_pdo_power(ada_srccaps[i], &max_ma, &max_mv, &unused); diff --git a/common/usb_pd_dual_role.c b/common/usb_pd_dual_role.c index 2ad0b99125..3c6aff6adf 100644 --- a/common/usb_pd_dual_role.c +++ b/common/usb_pd_dual_role.c @@ -61,6 +61,9 @@ int pd_find_pdo_index(uint32_t src_cap_cnt, const uint32_t * const src_caps, /* Get max power that is under our max voltage input */ for (i = 0; i < src_cap_cnt; i++) { + if (IS_ENABLED(CONFIG_USB_PD_ONLY_FIXED_PDOS) && + (src_caps[i] & PDO_TYPE_MASK) != PDO_TYPE_FIXED) + continue; /* its an unsupported Augmented PDO (PD3.0) */ if ((src_caps[i] & PDO_TYPE_MASK) == PDO_TYPE_AUGMENTED) continue; diff --git a/include/config.h b/include/config.h index a001e263b3..6ba50d3855 100644 --- a/include/config.h +++ b/include/config.h @@ -4820,6 +4820,12 @@ */ #undef CONFIG_USB_PD_MAX_SINGLE_SOURCE_CURRENT +/* + * Ignore all non-fixed PDOs received from a src_caps message. Enable this for + * boards (like servo_v4) which only support FIXED PDO types. + */ +#undef CONFIG_USB_PD_ONLY_FIXED_PDOS + /* * Total current in mA the board can supply to external devices through * USB-C ports diff --git a/test/build.mk b/test/build.mk index 36882c1b26..9dd576aa41 100644 --- a/test/build.mk +++ b/test/build.mk @@ -83,6 +83,7 @@ test-list-host += usb_pd_int test-list-host += usb_pd test-list-host += usb_pd_giveback test-list-host += usb_pd_rev30 +test-list-host += usb_pd_pdo_fixed test-list-host += usb_ppc test-list-host += usb_sm_framework_h3 test-list-host += usb_sm_framework_h2 @@ -200,6 +201,7 @@ usb_pd_int-y=usb_pd_int.o usb_pd-y=usb_pd.o usb_pd_giveback-y=usb_pd.o usb_pd_rev30-y=usb_pd.o +usb_pd_pdo_fixed-y=usb_pd_pdo_fixed_test.o usb_ppc-y=usb_ppc.o usb_sm_framework_h3-y=usb_sm_framework_h3.o usb_sm_framework_h2-y=usb_sm_framework_h3.o diff --git a/test/test_config.h b/test/test_config.h index eb78fa1995..d06db8252e 100644 --- a/test/test_config.h +++ b/test/test_config.h @@ -333,6 +333,17 @@ int ncp15wb_calculate_temp(uint16_t adc); #define CONFIG_SW_CRC #endif +#ifdef TEST_USB_PD_PDO_FIXED +#define CONFIG_USB_POWER_DELIVERY +#define CONFIG_USB_PD_TCPMV1 +#define CONFIG_USB_PD_PORT_MAX_COUNT 1 +#define CONFIG_USB_PD_TCPC +#define CONFIG_USB_PD_TCPM_STUB +#define CONFIG_SHA256 +#define CONFIG_SW_CRC +#define CONFIG_USB_PD_ONLY_FIXED_PDOS +#endif + #if defined(TEST_USB_SM_FRAMEWORK_H3) || \ defined(TEST_USB_SM_FRAMEWORK_H2) || \ defined(TEST_USB_SM_FRAMEWORK_H1) || \ diff --git a/test/usb_pd_pdo_fixed.tasklist b/test/usb_pd_pdo_fixed.tasklist new file mode 100644 index 0000000000..9a1e6b3e08 --- /dev/null +++ b/test/usb_pd_pdo_fixed.tasklist @@ -0,0 +1,10 @@ +/* Copyright 2021 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +/** + * See CONFIG_TASK_LIST in config.h for details. + */ +#define CONFIG_TEST_TASK_LIST + diff --git a/test/usb_pd_pdo_fixed_test.c b/test/usb_pd_pdo_fixed_test.c new file mode 100644 index 0000000000..ad247c3ba2 --- /dev/null +++ b/test/usb_pd_pdo_fixed_test.c @@ -0,0 +1,47 @@ +/* Copyright 2021 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + * + * Test USB common module. + */ +#include "test_util.h" +#include "usb_common.h" + +#define PDO_FIXED_FLAGS \ + (PDO_FIXED_DUAL_ROLE | PDO_FIXED_DATA_SWAP | PDO_FIXED_COMM_CAP) + +/* Test that a non-fixed PDO will never be selected by pd_find_pdo_index. */ +test_static int test_pd_find_pdo_index(void) +{ + const uint32_t pd_snk_pdo[] = { + PDO_FIXED(5000, 500, PDO_FIXED_FLAGS), + PDO_VAR(4750, PD_MAX_VOLTAGE_MV, PD_MAX_CURRENT_MA), + PDO_BATT(4750, PD_MAX_VOLTAGE_MV, PD_MAX_POWER_MW), + PDO_FIXED(9000, 3000, PDO_FIXED_FLAGS), + PDO_FIXED(12000, 3000, PDO_FIXED_FLAGS), + PDO_FIXED(20000, 3000, PDO_FIXED_FLAGS), + }; + const int pd_snk_pdo_cnt = ARRAY_SIZE(pd_snk_pdo); + uint32_t pdo; + + TEST_EQ(pd_find_pdo_index(pd_snk_pdo_cnt, pd_snk_pdo, 5000, &pdo), 0, + "%d"); + TEST_EQ(pd_find_pdo_index(pd_snk_pdo_cnt, pd_snk_pdo, 9000, &pdo), 3, + "%d"); + TEST_EQ(pd_find_pdo_index(pd_snk_pdo_cnt, pd_snk_pdo, 10000, &pdo), 3, + "%d"); + TEST_EQ(pd_find_pdo_index(pd_snk_pdo_cnt, pd_snk_pdo, 12000, &pdo), 4, + "%d"); + TEST_EQ(pd_find_pdo_index(pd_snk_pdo_cnt, pd_snk_pdo, 15000, &pdo), 4, + "%d"); + TEST_EQ(pd_find_pdo_index(pd_snk_pdo_cnt, pd_snk_pdo, 20000, &pdo), 5, + "%d"); + return EC_SUCCESS; +} + +void run_test(int argc, char **argv) +{ + RUN_TEST(test_pd_find_pdo_index); + + test_print_result(); +} diff --git a/zephyr/Kconfig.usbc b/zephyr/Kconfig.usbc index 9932de39c8..0dea4290e8 100644 --- a/zephyr/Kconfig.usbc +++ b/zephyr/Kconfig.usbc @@ -980,6 +980,13 @@ endif # PLATFORM_EC_USB_PD_TCPM_TCPCI endif # PLATFORM_EC_USBC_PPC +config PLATFORM_EC_USB_PD_ONLY_FIXED_PDOS + bool "Only support FIXED type PDOs" + default n + help + Ignore all non-fixed PDOs received from a src_caps message. Enable + this for boards (like servo_v4) which only support FIXED PDO types. + config PLATFORM_EC_USB_CHARGER bool "Support charging from a USB-C port" default y diff --git a/zephyr/shim/include/config_chip.h b/zephyr/shim/include/config_chip.h index 817adac338..303f3deb29 100644 --- a/zephyr/shim/include/config_chip.h +++ b/zephyr/shim/include/config_chip.h @@ -1418,4 +1418,9 @@ #define CONFIG_CMD_CHARGER_ADC_AMON_BMON #endif +#undef CONFIG_USB_PD_ONLY_FIXED_PDOS +#ifdef CONFIG_PLATFORM_EC_USB_PD_ONLY_FIXED_PDOS +#define CONFIG_USB_PD_ONLY_FIXED_PDOS +#endif + #endif /* __CROS_EC_CONFIG_CHIP_H */ -- cgit v1.2.1