pdchipinfo: add min firmware version to pdchipinfo

Add a new field to the pdchipinfo host command that exposes the minimum
required firmware version that we know about. This will allow factory
tests or automated test to compare this version to the current version
and fail.

BRANCH=none
BUG=b:116068318
TEST=with corresponding ectool change, min value is reported correctly

Change-Id: Idf338795c3fd6f9f95e51471d0f6a7a422901d52
Signed-off-by: Jett Rink <jettrink@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1240457
Reviewed-by: Justin TerAvest <teravest@chromium.org>

12 files changed, 102 insertions, 59 deletions

diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c
index aa9313a9ad..966d89bb57 100644
--- a/common/usb_pd_protocol.c
+++ b/common/usb_pd_protocol.c
@@ -2531,7 +2531,7 @@ void pd_task(void *u)
 	this_state = res ? PD_STATE_SUSPENDED : PD_DEFAULT_STATE(port);
 #ifndef CONFIG_USB_PD_TCPC
 	if (!res) {
-		struct ec_response_pd_chip_info *info;
+		struct ec_response_pd_chip_info_v1 *info;
 
 		if (tcpm_get_chip_info(port, 0, &info) == EC_SUCCESS) {
 			CPRINTS("TCPC p%d VID:0x%x PID:0x%x DID:0x%x FWV:0x%lx",
@@ -4773,7 +4773,7 @@ DECLARE_HOST_COMMAND(EC_CMD_USB_PD_DEV_INFO,
 static int hc_remote_pd_chip_info(struct host_cmd_handler_args *args)
 {
 	const struct ec_params_pd_chip_info *p = args->params;
-	struct ec_response_pd_chip_info *r = args->response, *info;
+	struct ec_response_pd_chip_info_v1 *info;
 
 	if (p->port >= CONFIG_USB_PD_PORT_COUNT)
 		return EC_RES_INVALID_PARAM;
@@ -4781,14 +4781,21 @@ static int hc_remote_pd_chip_info(struct host_cmd_handler_args *args)
 	if (tcpm_get_chip_info(p->port, p->renew, &info))
 		return EC_RES_ERROR;
 
-	memcpy(r, info, sizeof(*r));
-	args->response_size = sizeof(*r);
+	/*
+	 * Take advantage of the fact that v0 and v1 structs have the
+	 * same layout for v0 data. (v1 just appends data)
+	 */
+	args->response_size =
+		args->version ? sizeof(struct ec_response_pd_chip_info_v1)
+			      : sizeof(struct ec_response_pd_chip_info);
+
+	memcpy(args->response, info, args->response_size);
 
 	return EC_RES_SUCCESS;
 }
 DECLARE_HOST_COMMAND(EC_CMD_PD_CHIP_INFO,
 		     hc_remote_pd_chip_info,
-		     EC_VER_MASK(0));
+		     EC_VER_MASK(0) | EC_VER_MASK(1));
 #endif
 #endif
 
diff --git a/driver/tcpm/anx74xx.c b/driver/tcpm/anx74xx.c
index f9a4b09089..79af4c054b 100644
--- a/driver/tcpm/anx74xx.c
+++ b/driver/tcpm/anx74xx.c
@@ -746,7 +746,7 @@ static int anx74xx_tcpm_set_polarity(int port, int polarity)
 	return rv;
 }
 
-int anx74xx_tcpc_get_fw_version(int port, int *version)
+static int anx74xx_tcpc_get_fw_version(int port, int *version)
 {
 	return tcpc_read(port, ANX74XX_REG_FW_VERSION, version);
 }
@@ -1043,6 +1043,25 @@ static int anx74xx_tcpm_init(int port)
 	return EC_SUCCESS;
 }
 
+static int anx74xx_get_chip_info(int port, int renew,
+			struct ec_response_pd_chip_info_v1 **chip_info)
+{
+	int rv = tcpci_get_chip_info(port, renew, chip_info);
+	int val;
+
+	if (rv)
+		return rv;
+
+	rv = anx74xx_tcpc_get_fw_version(port, &val);
+
+	if (rv)
+		return rv;
+
+	(*chip_info)->fw_version_number = val;
+
+	return rv;
+}
+
 /*
  * Dissociate from the TCPC.
  */
@@ -1071,7 +1090,7 @@ const struct tcpm_drv anx74xx_tcpm_drv = {
 #ifdef CONFIG_USB_PD_DISCHARGE_TCPC
 	.tcpc_discharge_vbus	= &anx74xx_tcpc_discharge_vbus,
 #endif
-	.get_chip_info		= &tcpci_get_chip_info,
+	.get_chip_info		= &anx74xx_get_chip_info,
 #if defined(CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE) && \
 		defined(CONFIG_USB_PD_TCPC_LOW_POWER)
 	.drp_toggle		= &anx74xx_tcpc_drp_toggle,
diff --git a/driver/tcpm/anx74xx.h b/driver/tcpm/anx74xx.h
index 9bdaadadcd..a8f51232a5 100644
--- a/driver/tcpm/anx74xx.h
+++ b/driver/tcpm/anx74xx.h
@@ -211,7 +211,6 @@ extern const struct usb_mux_driver anx74xx_tcpm_usb_mux_driver;
 void anx74xx_tcpc_set_vbus(int port, int enable);
 void anx74xx_tcpc_update_hpd_status(int port, int hpd_lvl, int hpd_irq);
 void anx74xx_tcpc_clear_hpd_status(int port);
-int anx74xx_tcpc_get_fw_version(int port, int *version);
 
 #ifdef CONFIG_CMD_I2C_STRESS_TEST_TCPC
 extern struct i2c_stress_test_dev anx74xx_i2c_stress_test_dev;
diff --git a/driver/tcpm/it83xx.c b/driver/tcpm/it83xx.c
index 24097e1199..8fb6b985c0 100644
--- a/driver/tcpm/it83xx.c
+++ b/driver/tcpm/it83xx.c
@@ -554,9 +554,9 @@ static int it83xx_tcpm_transmit(int port,
 }
 
 static int it83xx_tcpm_get_chip_info(int port, int renew,
-			struct ec_response_pd_chip_info **chip_info)
+			struct ec_response_pd_chip_info_v1 **chip_info)
 {
-	static struct ec_response_pd_chip_info i;
+	static struct ec_response_pd_chip_info_v1 i;
 
 	*chip_info = &i;
 	i.vendor_id = USB_VID_ITE;
diff --git a/driver/tcpm/ps8xxx.c b/driver/tcpm/ps8xxx.c
index d58f3cde35..19d0c21396 100644
--- a/driver/tcpm/ps8xxx.c
+++ b/driver/tcpm/ps8xxx.c
@@ -87,7 +87,7 @@ void ps8xxx_tcpc_update_hpd_status(int port, int hpd_lvl, int hpd_irq)
 	hpd_deadline[port] = get_time().val + HPD_USTREAM_DEBOUNCE_LVL;
 }
 
-int ps8xxx_tcpc_get_fw_version(int port, int *version)
+static int ps8xxx_tcpc_get_fw_version(int port, int *version)
 {
 	return tcpc_read(port, FW_VER_REG, version);
 }
@@ -136,6 +136,26 @@ static int ps8xxx_tcpm_release(int port)
 	return tcpci_tcpm_release(port);
 }
 
+static int ps8xxx_get_chip_info(int port, int renew,
+			struct ec_response_pd_chip_info_v1 **chip_info)
+{
+	int val;
+	int rv = tcpci_get_chip_info(port, renew, chip_info);
+
+	if (rv)
+		return rv;
+
+	rv = ps8xxx_tcpc_get_fw_version(port, &val);
+
+	if (rv)
+		return rv;
+
+	(*chip_info)->fw_version_number = val;
+
+	return rv;
+}
+
+
 const struct tcpm_drv ps8xxx_tcpm_drv = {
 	.init			= &tcpci_tcpm_init,
 	.release		= &ps8xxx_tcpm_release,
@@ -162,7 +182,7 @@ const struct tcpm_drv ps8xxx_tcpm_drv = {
 	.set_snk_ctrl		= &tcpci_tcpm_set_snk_ctrl,
 	.set_src_ctrl		= &tcpci_tcpm_set_src_ctrl,
 #endif
-	.get_chip_info		= &tcpci_get_chip_info,
+	.get_chip_info		= &ps8xxx_get_chip_info,
 #ifdef CONFIG_USB_PD_TCPC_LOW_POWER
 	.enter_low_power_mode	= &tcpci_enter_low_power_mode,
 #endif
diff --git a/driver/tcpm/ps8xxx.h b/driver/tcpm/ps8xxx.h
index 6c66210982..1459bb4b3e 100644
--- a/driver/tcpm/ps8xxx.h
+++ b/driver/tcpm/ps8xxx.h
@@ -62,7 +62,6 @@
 
 extern const struct tcpm_drv ps8xxx_tcpm_drv;
 void ps8xxx_tcpc_update_hpd_status(int port, int hpd_lvl, int hpd_irq);
-int ps8xxx_tcpc_get_fw_version(int port, int *version);
 
 #ifdef CONFIG_CMD_I2C_STRESS_TEST_TCPC
 extern struct i2c_stress_test_dev ps8xxx_i2c_stress_test_dev;
diff --git a/driver/tcpm/tcpci.c b/driver/tcpm/tcpci.c
index 84e22bda06..1c1c04868d 100644
--- a/driver/tcpm/tcpci.c
+++ b/driver/tcpm/tcpci.c
@@ -587,21 +587,18 @@ void tcpci_tcpc_alert(int port)
 }
 
 /*
- * For PS8751, this function will fail if the chip is in low power mode.
- * PS8751 has to be woken up by reading a random register first then wait for
- * 10ms.
+ * This call will wake up the TCPC if it is in low power mode upon accessing the
+ * i2c bus (but the pd state machine should put it back into low power mode).
  *
- * This code doesn't have the wake-up read to avoid 10ms delay. Instead, we
- * call this function immediately after the chip is reset or initialized
- * because it'll gurantee the chip is awake. Once it's called, the chip info
- * will be stored in cache, which can be accessed by tcpm_get_chip_info without
- * worrying about chip states.
+ * Once it's called, the chip info will be stored in cache, which can be
+ * accessed by tcpm_get_chip_info without worrying about chip states.
  */
 int tcpci_get_chip_info(int port, int renew,
-			struct ec_response_pd_chip_info **chip_info)
+			struct ec_response_pd_chip_info_v1 **chip_info)
 {
-	static struct ec_response_pd_chip_info info[CONFIG_USB_PD_PORT_COUNT];
-	struct ec_response_pd_chip_info *i;
+	static struct ec_response_pd_chip_info_v1
+		info[CONFIG_USB_PD_PORT_COUNT];
+	struct ec_response_pd_chip_info_v1 *i;
 	int error;
 	int val;
 
@@ -634,30 +631,11 @@ int tcpci_get_chip_info(int port, int renew,
 		return error;
 	i->device_id = val;
 
-	switch (i->vendor_id) {
-#if  defined(CONFIG_USB_PD_TCPM_ANX3429) || \
-	defined(CONFIG_USB_PD_TCPM_ANX740X) || \
-	defined(CONFIG_USB_PD_TCPM_ANX741X)
-	case ANX74XX_VENDOR_ID:
-		error = anx74xx_tcpc_get_fw_version(port, &val);
-		break;
-#endif
-#if defined(CONFIG_USB_PD_TCPM_PS8751) || defined(CONFIG_USB_PD_TCPM_PS8805)
-	/* The PS8751 and PS8805 share the same vendor ID. */
-	case PS8XXX_VENDOR_ID:
-		error = ps8xxx_tcpc_get_fw_version(port, &val);
-		break;
-#endif
-	default:
-		/* Even if the chip doesn't implement get_fw_version, we
-		 * return success.*/
-		val = -1;
-		error = EC_SUCCESS;
-	}
-	if (error)
-		return error;
-	/* This may vary chip to chip. For now everything fits in this format */
-	i->fw_version_number = val;
+	/*
+	 * This varies chip to chip; more specific driver code is expected to
+	 * override this value if it can.
+	 */
+	i->fw_version_number = -1;
 
 	return EC_SUCCESS;
 }
diff --git a/driver/tcpm/tcpci.h b/driver/tcpm/tcpci.h
index 24c5d9c2f6..f5fe1a1a0b 100644
--- a/driver/tcpm/tcpci.h
+++ b/driver/tcpm/tcpci.h
@@ -155,7 +155,7 @@ int tcpci_tcpm_mux_init(int i2c_addr);
 int tcpci_tcpm_mux_set(int i2c_addr, mux_state_t mux_state);
 int tcpci_tcpm_mux_get(int i2c_addr, mux_state_t *mux_state);
 int tcpci_get_chip_info(int port, int renew,
-			struct ec_response_pd_chip_info **chip_info);
+			struct ec_response_pd_chip_info_v1 **chip_info);
 #ifdef CONFIG_USBC_PPC
 int tcpci_tcpm_set_snk_ctrl(int port, int enable);
 int tcpci_tcpm_set_src_ctrl(int port, int enable);
diff --git a/driver/tcpm/tcpm.h b/driver/tcpm/tcpm.h
index 02cdbe2cb6..fc0cd2791d 100644
--- a/driver/tcpm/tcpm.h
+++ b/driver/tcpm/tcpm.h
@@ -234,7 +234,7 @@ static inline int tcpc_i2c_write(const int port, const int addr,
 #endif
 
 static inline int tcpm_get_chip_info(int port, int renew,
-				     struct ec_response_pd_chip_info **info)
+				     struct ec_response_pd_chip_info_v1 **info)
 {
 	if (tcpc_config[port].drv->get_chip_info)
 		return tcpc_config[port].drv->get_chip_info(port, renew, info);
diff --git a/include/ec_commands.h b/include/ec_commands.h
index e6d02c8ee9..8ce663898c 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -4721,6 +4721,20 @@ struct __ec_align2 ec_response_pd_chip_info {
 	};
 };
 
+struct __ec_align2 ec_response_pd_chip_info_v1 {
+	uint16_t vendor_id;
+	uint16_t product_id;
+	uint16_t device_id;
+	union {
+		uint8_t fw_version_string[8];
+		uint64_t fw_version_number;
+	};
+	union {
+		uint8_t min_req_fw_version_string[8];
+		uint64_t min_req_fw_version_number;
+	};
+};
+
 /* Run RW signature verification and get status */
 #define EC_CMD_RWSIG_CHECK_STATUS	0x011C
 
diff --git a/include/usb_pd_tcpm.h b/include/usb_pd_tcpm.h
index 2d12b31ceb..00fe2ab958 100644
--- a/include/usb_pd_tcpm.h
+++ b/include/usb_pd_tcpm.h
@@ -218,7 +218,7 @@ struct tcpm_drv {
 	 * @return EC_SUCCESS or error
 	 */
 	int (*get_chip_info)(int port, int renew,
-			struct ec_response_pd_chip_info **info);
+			struct ec_response_pd_chip_info_v1 **info);
 
 #ifdef CONFIG_USBC_PPC
 	/**
diff --git a/util/ectool.c b/util/ectool.c
index 00b58b026e..2322376b36 100644
--- a/util/ectool.c
+++ b/util/ectool.c
@@ -7892,9 +7892,10 @@ int cmd_pd_control(int argc, char *argv[])
 int cmd_pd_chip_info(int argc, char *argv[])
 {
 	struct ec_params_pd_chip_info p;
-	struct ec_response_pd_chip_info r;
+	struct ec_response_pd_chip_info_v1 r;
 	char *e;
 	int rv;
+	int cmdver = 1;
 
 	if (argc < 2 || 3 < argc) {
 		fprintf(stderr, "Usage: %s <port> [renew(on/off)]\n", argv[0]);
@@ -7917,7 +7918,11 @@ int cmd_pd_chip_info(int argc, char *argv[])
 		p.renew = val;
 	}
 
-	rv = ec_command(EC_CMD_PD_CHIP_INFO, 0, &p, sizeof(p), &r, sizeof(r));
+	if (!ec_cmd_version_supported(EC_CMD_PD_CHIP_INFO, cmdver))
+		cmdver = 0;
+
+	rv = ec_command(EC_CMD_PD_CHIP_INFO, cmdver, &p, sizeof(p), &r,
+			sizeof(r));
 	if (rv < 0)
 		return rv;
 
@@ -7925,14 +7930,16 @@ int cmd_pd_chip_info(int argc, char *argv[])
 	printf("product_id: 0x%x\n", r.product_id);
 	printf("device_id: 0x%x\n", r.device_id);
 
-	switch (r.vendor_id) {
-	case ANX74XX_VENDOR_ID:
-	case PS8XXX_VENDOR_ID:
+	if (r.fw_version_number != -1)
 		printf("fw_version: 0x%" PRIx64 "\n", r.fw_version_number);
-		break;
-	default:
+	else
 		printf("fw_version: UNSUPPORTED\n");
-	}
+
+	if (cmdver >= 1)
+		printf("min_req_fw_version: 0x%" PRIx64 "\n",
+		       r.min_req_fw_version_number);
+	else
+		printf("min_req_fw_version: UNSUPPORTED\n");
 
 	return 0;
 }