4 files changed, 131 insertions, 44 deletions

diff --git a/driver/tcpm/tcpci.c b/driver/tcpm/tcpci.c
index 84fdbe8cbc..6c6e7a01c1 100644
--- a/driver/tcpm/tcpci.c
+++ b/driver/tcpm/tcpci.c
@@ -1375,67 +1375,82 @@ int tcpci_get_vbus_voltage(int port, int *vbus)
 	return EC_SUCCESS;
 }
 
-/*
- * 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).
- *
- * 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 live,
-			struct ec_response_pd_chip_info_v1 *chip_info)
+int tcpci_get_chip_info_mutable(
+	int port, int live, struct ec_response_pd_chip_info_v1 *const chip_info,
+	int (*const mutator)(int port, bool live,
+			     struct ec_response_pd_chip_info_v1 *cached))
 {
 	static struct ec_response_pd_chip_info_v1
 		cached_info[CONFIG_USB_PD_PORT_MAX_COUNT];
-	struct ec_response_pd_chip_info_v1 *i;
+	struct ec_response_pd_chip_info_v1 *info;
 	int error;
-	int val;
 
 	if (port >= board_get_usb_pd_port_count())
 		return EC_ERROR_INVAL;
 
-	i = &cached_info[port];
+	info = &cached_info[port];
+
+	/* Fetch live data if nothing is cached or live data was requested */
+	if (!info->vendor_id || live) {
+		int vendor_id, product_id, device_id;
 
-	/* If already cached && live data is not asked, return cached value */
-	if (i->vendor_id && !live) {
 		/*
-		 * If chip_info is NULL, chip info will be stored in cache and
-		 * can be read later by another call.
+		 * The cache is no longer valid because we're fetching. Avoid
+		 * storing the new vendor ID until we've actually succeeded so
+		 * future invocations won't see partial data and assume it's a
+		 * valid cache.
 		 */
-		if (chip_info)
-			memcpy(chip_info, i, sizeof(*i));
-
-		return EC_SUCCESS;
-	}
-
-	error = tcpc_read16(port, TCPC_REG_VENDOR_ID, &val);
-	if (error)
-		return error;
-	i->vendor_id = val;
+		info->vendor_id = 0;
+		error = tcpc_read16(port, TCPC_REG_VENDOR_ID, &vendor_id);
+		if (error)
+			return error;
 
-	error = tcpc_read16(port, TCPC_REG_PRODUCT_ID, &val);
-	if (error)
-		return error;
-	i->product_id = val;
+		error = tcpc_read16(port, TCPC_REG_PRODUCT_ID, &product_id);
+		if (error)
+			return error;
+		info->product_id = product_id;
 
-	error = tcpc_read16(port, TCPC_REG_BCD_DEV, &val);
-	if (error)
-		return error;
-	i->device_id = val;
+		error = tcpc_read16(port, TCPC_REG_BCD_DEV, &device_id);
+		if (error)
+			return error;
+		info->device_id = device_id;
 
+		/*
+		 * This varies chip to chip; more specific driver code is
+		 * expected to override this value if it can by providing a
+		 * mutator.
+		 */
+		info->fw_version_number = -1;
+
+		info->vendor_id = vendor_id;
+		if (mutator != NULL) {
+			error = mutator(port, live, info);
+			if (error) {
+				/*
+				 * Mutator needs to have a complete view, but if
+				 * it fails the cache is invalidated.
+				 */
+				info->vendor_id = 0;
+				return error;
+			}
+		}
+	}
 	/*
-	 * This varies chip to chip; more specific driver code is expected to
-	 * override this value if it can.
+	 * If chip_info is NULL, this invocation will ensure the cache is fresh
+	 * but return nothing.
 	 */
-	i->fw_version_number = -1;
-
-	/* Copy the cached value to return if chip_info is not NULL */
 	if (chip_info)
-		memcpy(chip_info, i, sizeof(*i));
+		memcpy(chip_info, info, sizeof(*info));
 
 	return EC_SUCCESS;
 }
 
+int tcpci_get_chip_info(int port, int live,
+			struct ec_response_pd_chip_info_v1 *chip_info)
+{
+	return tcpci_get_chip_info_mutable(port, live, chip_info, NULL);
+}
+
 /*
  * Dissociate from the TCPC.
  */
diff --git a/include/driver/tcpm/tcpci.h b/include/driver/tcpm/tcpci.h
index 4879f7dad1..eb2b4e7f9f 100644
--- a/include/driver/tcpm/tcpci.h
+++ b/include/driver/tcpm/tcpci.h
@@ -340,8 +340,46 @@ int tcpci_tcpm_mux_set(const struct usb_mux *me, mux_state_t mux_state,
 		       bool *ack_required);
 int tcpci_tcpm_mux_get(const struct usb_mux *me, mux_state_t *mux_state);
 int tcpci_tcpm_mux_enter_low_power(const struct usb_mux *me);
+
+/**
+ * Get the TCPC chip information (chip IDs, etc) for the given port.
+ *
+ * The returned value is cached internally, so subsequent calls to this function
+ * will not access the TCPC. If live is true, data will be fetched from the TCPC
+ * regardless of whether any cached data is available.
+ *
+ * If chip_info is NULL, this will ensure the cache is up to date but avoid
+ * writing the output chip_info.
+ *
+ * If the TCPC is accessed (live data is retrieved), this will wake the chip up
+ * from low power mode on I2C access. It is expected that the USB-PD state
+ * machine will return it to low power mode as appropriate afterward.
+ *
+ * Returns EC_SUCCESS or an error; chip_info is not updated on error.
+ */
 int tcpci_get_chip_info(int port, int live,
 			struct ec_response_pd_chip_info_v1 *chip_info);
+
+/**
+ * Equivalent to tcpci_get_chip_info, but allows the caller to modify the cache
+ * if new data is fetched.
+ *
+ * If mutator is non-NULL and data is read from the TCPC (either because live
+ * data is requested or nothing was cached), then it is called with a pointer
+ * to the cached information for the port. It can then make changes to the
+ * cached data (for example correcting IDs that are known to be reported
+ * incorrectly by some chips, possibly requiring more communication with the
+ * TCPC). Any error returned by mutator causes this function to return with the
+ * same error.
+ *
+ * If mutator writes through the cached data pointer, those changes will be
+ * retained until live data is requested again.
+ */
+int tcpci_get_chip_info_mutable(
+	int port, int live, struct ec_response_pd_chip_info_v1 *chip_info,
+	int (*mutator)(int port, bool live,
+		       struct ec_response_pd_chip_info_v1 *cached));
+
 int tcpci_get_vbus_voltage(int port, int *vbus);
 bool tcpci_tcpm_get_snk_ctrl(int port);
 int tcpci_tcpm_set_snk_ctrl(int port, int enable);
diff --git a/zephyr/test/drivers/default/src/tcpci.c b/zephyr/test/drivers/default/src/tcpci.c
index f648b43a96..90d5d3fb65 100644
--- a/zephyr/test/drivers/default/src/tcpci.c
+++ b/zephyr/test/drivers/default/src/tcpci.c
@@ -200,6 +200,11 @@ ZTEST(tcpci, test_generic_tcpci_get_chip_info)
 		emul_tcpci_generic_get_i2c_common_data(emul);
 
 	test_tcpci_get_chip_info(emul, common_data, USBC_PORT_C0);
+
+	zassert_equal(EC_ERROR_INVAL,
+		      tcpci_get_chip_info(board_get_usb_pd_port_count(), false,
+					  NULL),
+		      "get_chip_info should return INVAL for an invalid port");
 }
 
 /** Test TCPCI enter low power mode */
diff --git a/zephyr/test/drivers/default/src/tcpci_test_common.c b/zephyr/test/drivers/default/src/tcpci_test_common.c
index 06f03d4834..01f1cd944c 100644
--- a/zephyr/test/drivers/default/src/tcpci_test_common.c
+++ b/zephyr/test/drivers/default/src/tcpci_test_common.c
@@ -900,6 +900,19 @@ void test_tcpci_drp_toggle(const struct emul *emul,
 			TCPC_REG_COMMAND_LOOK4CONNECTION);
 }
 
+static int test_tcpci_get_chip_info_mutator_device_id(
+	int port, bool live, struct ec_response_pd_chip_info_v1 *cached)
+{
+	cached->device_id = 0xbeef;
+	return EC_SUCCESS;
+}
+
+static int test_tcpci_get_chip_info_mutator_fail(
+	int port, bool live, struct ec_response_pd_chip_info_v1 *cached)
+{
+	return EC_ERROR_UNCHANGED;
+}
+
 /** Test TCPCI get chip info */
 void test_tcpci_get_chip_info(const struct emul *emul,
 			      struct i2c_common_emul_data *common_data,
@@ -913,13 +926,16 @@ void test_tcpci_get_chip_info(const struct emul *emul,
 	i2c_common_emul_set_read_fail_reg(common_data, TCPC_REG_VENDOR_ID);
 	zassert_equal(EC_ERROR_INVAL, drv->get_chip_info(port, 1, &info));
 
-	/* Test error on failed product id get */
+	/*
+	 * Test error on failed product id get. Cache should not be valid
+	 * because the previous call to get_chip_info failed.
+	 */
 	i2c_common_emul_set_read_fail_reg(common_data, TCPC_REG_PRODUCT_ID);
-	zassert_equal(EC_ERROR_INVAL, drv->get_chip_info(port, 1, &info));
+	zassert_equal(EC_ERROR_INVAL, drv->get_chip_info(port, 0, &info));
 
 	/* Test error on failed BCD get */
-	i2c_common_emul_set_read_fail_reg(common_data, TCPC_REG_VENDOR_ID);
-	zassert_equal(EC_ERROR_INVAL, drv->get_chip_info(port, 1, &info));
+	i2c_common_emul_set_read_fail_reg(common_data, TCPC_REG_BCD_DEV);
+	zassert_equal(EC_ERROR_INVAL, drv->get_chip_info(port, 0, &info));
 	i2c_common_emul_set_read_fail_reg(common_data,
 					  I2C_COMMON_EMUL_NO_FAIL_REG);
 
@@ -948,6 +964,19 @@ void test_tcpci_get_chip_info(const struct emul *emul,
 	zassert_equal(vendor, info.vendor_id);
 	zassert_equal(product, info.product_id);
 	zassert_equal(bcd, info.device_id);
+
+	/* Test providing a callback to modify cached data. */
+	zassert_equal(EC_SUCCESS,
+		      tcpci_get_chip_info_mutable(
+			      port, 1, &info,
+			      test_tcpci_get_chip_info_mutator_device_id));
+	zassert_equal(0xbeef, info.device_id);
+
+	/* Errors from the mutator get bubbled up. */
+	zassert_equal(EC_ERROR_UNCHANGED,
+		      tcpci_get_chip_info_mutable(
+			      port, 1, &info,
+			      test_tcpci_get_chip_info_mutator_fail));
 }
 
 /** Test TCPCI enter low power mode */