ppc: cleanup ppc

Allow limited PPC chips to default to EC_ERROR_UNIMPLEMENTED
for functions in the driver that are not needed.

BUG=b:138599218
BRANCH=none
TEST=make buildall -j

Change-Id: I5242ef285eb277c06d516ab09f7a74f76d7d34b2
Signed-off-by: Denis Brockus <dbrockus@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1829405
Reviewed-by: Edward Hill <ecgh@chromium.org>

6 files changed, 365 insertions, 37 deletions

diff --git a/common/usbc_ppc.c b/common/usbc_ppc.c
index 60a514d0f3..4274443a3d 100644
--- a/common/usbc_ppc.c
+++ b/common/usbc_ppc.c
@@ -13,8 +13,13 @@
 #include "usbc_ppc.h"
 #include "util.h"
 
+#ifndef TEST_BUILD
 #define CPRINTF(format, args...) cprintf(CC_USBPD, format, ## args)
 #define CPRINTS(format, args...) cprints(CC_USBPD, format, ## args)
+#else
+#define CPRINTF(args...)
+#define CPRINTS(args...)
+#endif
 
 /*
  * A per-port table that indicates how many VBUS overcurrent events have
@@ -29,24 +34,32 @@ static uint32_t connected_ports;
 
 int ppc_init(int port)
 {
-	int rv;
+	int rv = EC_ERROR_UNIMPLEMENTED;
+	const struct ppc_config_t *ppc;
 
-	if (port >= ppc_cnt)
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
 		return EC_ERROR_INVAL;
+	}
 
-	rv = ppc_chips[port].drv->init(port);
-	if (rv)
-		CPRINTS("p%d: PPC init failed! (%d)", port, rv);
-	else
-		CPRINTS("p%d: PPC init'd.", port);
+	ppc = &ppc_chips[port];
+	if (ppc->drv->init) {
+		rv = ppc->drv->init(port);
+		if (rv)
+			CPRINTS("p%d: PPC init failed! (%d)", port, rv);
+		else
+			CPRINTS("p%d: PPC init'd.", port);
+	}
 
 	return rv;
 }
 
 int ppc_add_oc_event(int port)
 {
-	if ((port < 0) || (port >= ppc_cnt))
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
 		return EC_ERROR_INVAL;
+	}
 
 	oc_event_cnt_tbl[port]++;
 
@@ -79,8 +92,10 @@ DECLARE_DEFERRED(clear_oc_tbl);
 
 int ppc_clear_oc_event_counter(int port)
 {
-	if ((port < 0) || (port >= ppc_cnt))
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
 		return EC_ERROR_INVAL;
+	}
 
 	/*
 	 * If we are clearing our event table in quick succession, we may be in
@@ -98,44 +113,80 @@ int ppc_clear_oc_event_counter(int port)
 
 int ppc_is_sourcing_vbus(int port)
 {
+	int rv = EC_ERROR_UNIMPLEMENTED;
+	const struct ppc_config_t *ppc;
+
 	if ((port < 0) || (port >= ppc_cnt)) {
 		CPRINTS("%s(%d) Invalid port!", __func__, port);
-		return 0;
+		return EC_ERROR_INVAL;
 	}
 
-	return ppc_chips[port].drv->is_sourcing_vbus(port);
+	ppc = &ppc_chips[port];
+	if (ppc->drv->is_sourcing_vbus)
+		rv = ppc->drv->is_sourcing_vbus(port);
+
+	return rv;
 }
 
 #ifdef CONFIG_USBC_PPC_POLARITY
 int ppc_set_polarity(int port, int polarity)
 {
-	if ((port < 0) || (port >= ppc_cnt))
+	int rv = EC_ERROR_UNIMPLEMENTED;
+	const struct ppc_config_t *ppc;
+
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
 		return EC_ERROR_INVAL;
+	}
+
+	ppc = &ppc_chips[port];
+	if (ppc->drv->set_polarity)
+		rv = ppc->drv->set_polarity(port, polarity);
 
-	return ppc_chips[port].drv->set_polarity(port, polarity);
+	return rv;
 }
 #endif
 
 int ppc_set_vbus_source_current_limit(int port, enum tcpc_rp_value rp)
 {
-	if ((port < 0) || (port >= ppc_cnt))
+	int rv = EC_ERROR_UNIMPLEMENTED;
+	const struct ppc_config_t *ppc;
+
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
 		return EC_ERROR_INVAL;
+	}
 
-	return ppc_chips[port].drv->set_vbus_source_current_limit(port, rp);
+	ppc = &ppc_chips[port];
+	if (ppc->drv->set_vbus_source_current_limit)
+		rv = ppc->drv->set_vbus_source_current_limit(port, rp);
+
+	return rv;
 }
 
 int ppc_discharge_vbus(int port, int enable)
 {
-	if ((port < 0) || (port >= ppc_cnt))
+	int rv = EC_ERROR_UNIMPLEMENTED;
+	const struct ppc_config_t *ppc;
+
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
 		return EC_ERROR_INVAL;
+	}
+
+	ppc = &ppc_chips[port];
+	if (ppc->drv->discharge_vbus)
+		rv = ppc->drv->discharge_vbus(port, enable);
 
-	return ppc_chips[port].drv->discharge_vbus(port, enable);
+	return rv;
 }
 
 int ppc_is_port_latched_off(int port)
 {
-	if ((port < 0) || (port >= ppc_cnt))
-		return 0;
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
+		return EC_ERROR_INVAL;
+	}
 
 	return oc_event_cnt_tbl[port] >= PPC_OC_CNT_THRESH;
 }
@@ -143,18 +194,32 @@ int ppc_is_port_latched_off(int port)
 #ifdef CONFIG_USBC_PPC_SBU
 int ppc_set_sbu(int port, int enable)
 {
-	if ((port < 0) || (port >= ppc_cnt))
+	int rv = EC_ERROR_UNIMPLEMENTED;
+	const struct ppc_config_t *ppc;
+
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
 		return EC_ERROR_INVAL;
+	}
 
-	return ppc_chips[port].drv->set_sbu(port, enable);
+	ppc = &ppc_chips[port];
+	if (ppc->drv->set_sbu)
+		rv = ppc->drv->set_sbu(port, enable);
+
+	return rv;
 }
 #endif /* defined(CONFIG_USBC_PPC_SBU) */
 
 #ifdef CONFIG_USBC_PPC_VCONN
 int ppc_set_vconn(int port, int enable)
 {
-	if ((port < 0) || (port >= ppc_cnt))
+	int rv = EC_ERROR_UNIMPLEMENTED;
+	const struct ppc_config_t *ppc;
+
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
 		return EC_ERROR_INVAL;
+	}
 
 	/*
 	 * Check our OC event counter.  If we've exceeded our threshold, then
@@ -165,12 +230,21 @@ int ppc_set_vconn(int port, int enable)
 	if (enable && ppc_is_port_latched_off(port))
 		return EC_ERROR_ACCESS_DENIED;
 
-	return ppc_chips[port].drv->set_vconn(port, enable);
+	ppc = &ppc_chips[port];
+	if (ppc->drv->set_vconn)
+		rv = ppc->drv->set_vconn(port, enable);
+
+	return rv;
 }
 #endif
 
 void ppc_sink_is_connected(int port, int is_connected)
 {
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
+		return;
+	}
+
 	if (is_connected)
 		atomic_or(&connected_ports, 1 << port);
 	else
@@ -179,31 +253,47 @@ void ppc_sink_is_connected(int port, int is_connected)
 
 int ppc_vbus_sink_enable(int port, int enable)
 {
-	if ((port < 0) || (port >= ppc_cnt))
+	int rv = EC_ERROR_UNIMPLEMENTED;
+	const struct ppc_config_t *ppc;
+
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
 		return EC_ERROR_INVAL;
+	}
 
-	return ppc_chips[port].drv->vbus_sink_enable(port, enable);
+	ppc = &ppc_chips[port];
+	if (ppc->drv->vbus_sink_enable)
+		rv = ppc->drv->vbus_sink_enable(port, enable);
+
+	return rv;
 }
 
 int ppc_enter_low_power_mode(int port)
 {
+	int rv = EC_ERROR_UNIMPLEMENTED;
 	const struct ppc_config_t *ppc;
 
-	if ((port < 0) || (port >= ppc_cnt))
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
 		return EC_ERROR_INVAL;
+	}
 
 	ppc = &ppc_chips[port];
-
 	if (ppc->drv->enter_low_power_mode)
-		return ppc->drv->enter_low_power_mode(port);
-	else
-		return EC_ERROR_UNIMPLEMENTED;
+		rv = ppc->drv->enter_low_power_mode(port);
+
+	return rv;
 }
 
 int ppc_vbus_source_enable(int port, int enable)
 {
-	if ((port < 0) || (port >= ppc_cnt))
+	int rv = EC_ERROR_UNIMPLEMENTED;
+	const struct ppc_config_t *ppc;
+
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
 		return EC_ERROR_INVAL;
+	}
 
 	/*
 	 * Check our OC event counter.  If we've exceeded our threshold, then
@@ -214,18 +304,30 @@ int ppc_vbus_source_enable(int port, int enable)
 	if (enable && ppc_is_port_latched_off(port))
 		return EC_ERROR_ACCESS_DENIED;
 
-	return ppc_chips[port].drv->vbus_source_enable(port, enable);
+	ppc = &ppc_chips[port];
+	if (ppc->drv->vbus_source_enable)
+		rv = ppc->drv->vbus_source_enable(port, enable);
+
+	return rv;
 }
 
 #ifdef CONFIG_USB_PD_VBUS_DETECT_PPC
 int ppc_is_vbus_present(int port)
 {
+	int rv = EC_ERROR_UNIMPLEMENTED;
+	const struct ppc_config_t *ppc;
+
 	if ((port < 0) || (port >= ppc_cnt)) {
 		CPRINTS("%s(%d) Invalid port!", __func__, port);
-		return 0;
+		return EC_ERROR_INVAL;
 	}
 
-	return ppc_chips[port].drv->is_vbus_present(port);
+	ppc = &ppc_chips[port];
+
+	if (ppc->drv->is_vbus_present)
+		rv = ppc->drv->is_vbus_present(port);
+
+	return rv;
 }
 #endif /* defined(CONFIG_USB_PD_VBUS_DETECT_PPC) */
 
@@ -233,15 +335,23 @@ int ppc_is_vbus_present(int port)
 static int command_ppc_dump(int argc, char **argv)
 {
 	int port;
+	int rv = EC_ERROR_UNIMPLEMENTED;
+	const struct ppc_config_t *ppc;
 
 	if (argc < 2)
 		return EC_ERROR_PARAM_COUNT;
 
 	port = atoi(argv[1]);
-	if (port >= ppc_cnt)
-		return EC_ERROR_PARAM1;
+	if ((port < 0) || (port >= ppc_cnt)) {
+		CPRINTS("%s(%d) Invalid port!", __func__, port);
+		return EC_ERROR_INVAL;
+	}
 
-	return ppc_chips[port].drv->reg_dump(port);
+	ppc = &ppc_chips[port];
+	if (ppc->drv->reg_dump)
+		rv = ppc->drv->reg_dump(port);
+
+	return rv;
 }
 DECLARE_CONSOLE_COMMAND(ppc_dump, command_ppc_dump, "<Type-C port>",
 			"dump the PPC regs");
diff --git a/include/usbc_ppc.h b/include/usbc_ppc.h
index d553c2a767..efbfbe4b6c 100644
--- a/include/usbc_ppc.h
+++ b/include/usbc_ppc.h
@@ -23,6 +23,11 @@
  */
 #define PPC_OC_COOLDOWN_DELAY_US (2 * SECOND)
 
+/*
+ * NOTE: The pointers to functions in the ppc_drv structure can now be NULL
+ * which will indicate and return NOT_IMPLEMENTED from the main calling
+ * function
+ */
 struct ppc_drv {
 	/**
 	 * Initialize the PPC.
diff --git a/test/build.mk b/test/build.mk
index ea0fef7fc1..65a28fc3f6 100644
--- a/test/build.mk
+++ b/test/build.mk
@@ -70,6 +70,7 @@ test-list-host += uptime
 test-list-host += usb_pd
 test-list-host += usb_pd_giveback
 test-list-host += usb_pd_rev30
+test-list-host += usb_ppc
 test-list-host += usb_sm_framework_h3
 test-list-host += usb_sm_framework_h2
 test-list-host += usb_sm_framework_h1
@@ -145,6 +146,7 @@ uptime-y=uptime.o
 usb_pd-y=usb_pd.o
 usb_pd_giveback-y=usb_pd.o
 usb_pd_rev30-y=usb_pd.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
 usb_sm_framework_h1-y=usb_sm_framework_h3.o
diff --git a/test/test_config.h b/test/test_config.h
index b83d834056..c422143112 100644
--- a/test/test_config.h
+++ b/test/test_config.h
@@ -335,6 +335,15 @@ int ncp15wb_calculate_temp(uint16_t adc);
 #endif
 #endif /* TEST_USB_PD || TEST_USB_PD_GIVEBACK || TEST_USB_PD_REV30 */
 
+#ifdef TEST_USB_PPC
+#define CONFIG_USB_PD_PORT_COUNT 1
+#define CONFIG_USB_PD_VBUS_DETECT_PPC
+#define CONFIG_USBC_PPC
+#define CONFIG_USBC_PPC_POLARITY
+#define CONFIG_USBC_PPC_SBU
+#define CONFIG_USBC_PPC_VCONN
+#endif
+
 #if defined(TEST_CHARGE_MANAGER) || defined(TEST_CHARGE_MANAGER_DRP_CHARGING)
 #define CONFIG_CHARGE_MANAGER
 #define CONFIG_USB_PD_DUAL_ROLE
diff --git a/test/usb_ppc.c b/test/usb_ppc.c
new file mode 100644
index 0000000000..cf2a971a87
--- /dev/null
+++ b/test/usb_ppc.c
@@ -0,0 +1,193 @@
+/* Copyright 2019 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 PD module.
+ */
+#include "common.h"
+#include "console.h"
+#include "crc.h"
+#include "task.h"
+#include "test_util.h"
+#include "timer.h"
+#include "usbc_ppc.h"
+#include "util.h"
+
+const struct ppc_drv null_drv = {
+	.init = NULL,
+	.is_sourcing_vbus = NULL,
+	.vbus_sink_enable = NULL,
+	.vbus_source_enable = NULL,
+	.set_polarity = NULL,
+	.set_vbus_source_current_limit = NULL,
+	.discharge_vbus = NULL,
+	.set_sbu = NULL,
+	.set_vconn = NULL,
+	.is_vbus_present = NULL,
+	.enter_low_power_mode = NULL,
+};
+
+struct ppc_config_t ppc_chips[] = {
+	[0] = {
+		.drv = &null_drv
+	},
+};
+unsigned int ppc_cnt = ARRAY_SIZE(ppc_chips);
+
+const struct tcpc_config_t tcpc_config[] = {
+	[0] = {
+	},
+};
+
+static int test_ppc_init(void)
+{
+	int rv;
+
+	rv = ppc_init(1);
+	TEST_ASSERT(rv == EC_ERROR_INVAL);
+	rv = ppc_init(0);
+	TEST_ASSERT(rv == EC_ERROR_UNIMPLEMENTED);
+
+	return EC_SUCCESS;
+}
+
+static int test_ppc_is_sourcing_vbus(void)
+{
+	int rv;
+
+	rv = ppc_is_sourcing_vbus(1);
+	TEST_ASSERT(rv == EC_ERROR_INVAL);
+	rv = ppc_is_sourcing_vbus(0);
+	TEST_ASSERT(rv == EC_ERROR_UNIMPLEMENTED);
+
+	return EC_SUCCESS;
+}
+
+static int test_ppc_set_polarity(void)
+{
+	int rv;
+
+	rv = ppc_set_polarity(1, 0);
+	TEST_ASSERT(rv == EC_ERROR_INVAL);
+	rv = ppc_set_polarity(0, 0);
+	TEST_ASSERT(rv == EC_ERROR_UNIMPLEMENTED);
+
+	return EC_SUCCESS;
+}
+
+static int test_ppc_set_vbus_source_current_limit(void)
+{
+	int rv;
+
+	rv = ppc_set_vbus_source_current_limit(1, 0);
+	TEST_ASSERT(rv == EC_ERROR_INVAL);
+	rv = ppc_set_vbus_source_current_limit(0, 0);
+	TEST_ASSERT(rv == EC_ERROR_UNIMPLEMENTED);
+
+	return EC_SUCCESS;
+}
+
+static int test_ppc_set_sbu(void)
+{
+	int rv;
+
+	rv = ppc_set_sbu(1, 0);
+	TEST_ASSERT(rv == EC_ERROR_INVAL);
+	rv = ppc_set_sbu(0, 0);
+	TEST_ASSERT(rv == EC_ERROR_UNIMPLEMENTED);
+
+	return EC_SUCCESS;
+}
+
+static int test_ppc_set_vconn(void)
+{
+	int rv;
+
+	rv = ppc_set_vconn(1, 0);
+	TEST_ASSERT(rv == EC_ERROR_INVAL);
+	rv = ppc_set_vconn(0, 0);
+	TEST_ASSERT(rv == EC_ERROR_UNIMPLEMENTED);
+
+	return EC_SUCCESS;
+}
+
+static int test_ppc_discharge_vbus(void)
+{
+	int rv;
+
+	rv = ppc_discharge_vbus(1, 0);
+	TEST_ASSERT(rv == EC_ERROR_INVAL);
+	rv = ppc_discharge_vbus(0, 0);
+	TEST_ASSERT(rv == EC_ERROR_UNIMPLEMENTED);
+
+	return EC_SUCCESS;
+}
+
+static int test_ppc_vbus_sink_enable(void)
+{
+	int rv;
+
+	rv = ppc_vbus_sink_enable(1, 0);
+	TEST_ASSERT(rv == EC_ERROR_INVAL);
+	rv = ppc_vbus_sink_enable(0, 0);
+	TEST_ASSERT(rv == EC_ERROR_UNIMPLEMENTED);
+
+	return EC_SUCCESS;
+}
+
+static int test_ppc_enter_low_power_mode(void)
+{
+	int rv;
+
+	rv = ppc_enter_low_power_mode(1);
+	TEST_ASSERT(rv == EC_ERROR_INVAL);
+	rv = ppc_enter_low_power_mode(0);
+	TEST_ASSERT(rv == EC_ERROR_UNIMPLEMENTED);
+
+	return EC_SUCCESS;
+}
+
+static int test_ppc_vbus_source_enable(void)
+{
+	int rv;
+
+	rv = ppc_vbus_source_enable(1, 0);
+	TEST_ASSERT(rv == EC_ERROR_INVAL);
+	rv = ppc_vbus_source_enable(0, 0);
+	TEST_ASSERT(rv == EC_ERROR_UNIMPLEMENTED);
+
+	return EC_SUCCESS;
+}
+
+static int test_ppc_is_vbus_present(void)
+{
+	int rv;
+
+	rv = ppc_is_vbus_present(1);
+	TEST_ASSERT(rv == EC_ERROR_INVAL);
+	rv = ppc_is_vbus_present(0);
+	TEST_ASSERT(rv == EC_ERROR_UNIMPLEMENTED);
+
+	return EC_SUCCESS;
+}
+
+
+
+void run_test(void)
+{
+	test_reset();
+
+	RUN_TEST(test_ppc_init);
+	RUN_TEST(test_ppc_is_sourcing_vbus);
+	RUN_TEST(test_ppc_set_polarity);
+	RUN_TEST(test_ppc_set_vbus_source_current_limit);
+	RUN_TEST(test_ppc_set_sbu);
+	RUN_TEST(test_ppc_set_vconn);
+	RUN_TEST(test_ppc_discharge_vbus);
+	RUN_TEST(test_ppc_vbus_sink_enable);
+	RUN_TEST(test_ppc_enter_low_power_mode);
+	RUN_TEST(test_ppc_vbus_source_enable);
+	RUN_TEST(test_ppc_is_vbus_present);
+
+	test_print_result();
+}
diff --git a/test/usb_ppc.tasklist b/test/usb_ppc.tasklist
new file mode 100644
index 0000000000..9fc1a80f4d
--- /dev/null
+++ b/test/usb_ppc.tasklist
@@ -0,0 +1,9 @@
+/* Copyright 2019 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