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 <jbettis@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2809353
Tested-by: Jeremy Bettis <jbettis@chromium.org>
Commit-Queue: Keith Short <keithshort@chromium.org>
Reviewed-by: Keith Short <keithshort@chromium.org>

12 files changed, 97 insertions, 4 deletions

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
@@ -4821,6 +4821,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 */