TCPMv2: Add SOP minor revision to EC_CMD_TYPEC_STATUS

EC_CMD_TYPEC_STATUS is used to send data about USB-C partners and cables
to the kernel where it can then be exposed to userspace. Currently,
EC_CMD_TYPEC_STATUS only assigns major PD revision to sop_revision
causing the minor revision to appear as "0" regardless of the devices
actual minor PD revision. This CL adds minor revision assignment to
EC_CMD_TYPEC_STATUS so that it can be correctly reported by the kernel.

BUG=b:261736720
BRANCH=None
TEST="cat /sys/class/typec/port0-partner/usb_power_delivery_revision"
for USB PD 2.0, 3.0 and 3.1 devices.

Signed-off-by: Jameson Thies <jthies@google.com>
Change-Id: I8c1858d6e6d577628b373d16f9aabdfd15a0e3c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4163826
Reviewed-by: Diana Z <dzigterman@chromium.org>
Code-Coverage: Zoss <zoss-cl-coverage@prod.google.com>

8 files changed, 64 insertions, 20 deletions

diff --git a/common/usb_common.c b/common/usb_common.c
index ff8b39f47c..a83efb7a63 100644
--- a/common/usb_common.c
+++ b/common/usb_common.c
@@ -866,7 +866,7 @@ int pd_send_alert_msg(int port, uint32_t ado)
 	 * ADO before sending to a USB PD 3.0 partner and block the
 	 * message if the ADO is empty.
 	 */
-	partner_rmdo = pe_get_partner_rmdo(port);
+	partner_rmdo = pd_get_partner_rmdo(port);
 	if (partner_rmdo.major_rev == 0) {
 		ado &= ~(ADO_EXTENDED_ALERT_EVENT |
 			 ADO_EXTENDED_ALERT_EVENT_TYPE);
diff --git a/common/usbc/usb_pd_dpm.c b/common/usbc/usb_pd_dpm.c
index 0c2bfe3b25..fc01783b87 100644
--- a/common/usbc/usb_pd_dpm.c
+++ b/common/usbc/usb_pd_dpm.c
@@ -1041,7 +1041,7 @@ int dpm_get_status_msg(int port, uint8_t *msg, uint32_t *len)
 	/* Power Status */
 	sdb.power_status = 0x0;
 
-	partner_rmdo = pe_get_partner_rmdo(port);
+	partner_rmdo = pd_get_partner_rmdo(port);
 	if ((partner_rmdo.major_rev == 3 && partner_rmdo.minor_rev >= 1) ||
 	    partner_rmdo.major_rev > 3) {
 		/* USB PD Rev 3.1: 6.5.2 Status Message */
diff --git a/common/usbc/usb_pd_host.c b/common/usbc/usb_pd_host.c
index bf891cf5ed..2c08aee302 100644
--- a/common/usbc/usb_pd_host.c
+++ b/common/usbc/usb_pd_host.c
@@ -210,10 +210,23 @@ static enum ec_status hc_typec_status(struct host_cmd_handler_args *args)
 
 	r->events = pd_get_events(p->port);
 
-	r->sop_revision = r->sop_connected ?
-				  PD_STATUS_REV_SET_MAJOR(
-					  pd_get_rev(p->port, TCPCI_MSG_SOP)) :
-				  0;
+	if (pd_get_partner_rmdo(p->port).major_rev != 0) {
+		r->sop_revision =
+			PD_STATUS_RMDO_REV_SET_MAJOR(
+				pd_get_partner_rmdo(p->port).major_rev) |
+			PD_STATUS_RMDO_REV_SET_MINOR(
+				pd_get_partner_rmdo(p->port).minor_rev) |
+			PD_STATUS_RMDO_VER_SET_MAJOR(
+				pd_get_partner_rmdo(p->port).major_ver) |
+			PD_STATUS_RMDO_VER_SET_MINOR(
+				pd_get_partner_rmdo(p->port).minor_ver);
+	} else if (r->sop_connected) {
+		r->sop_revision = PD_STATUS_REV_SET_MAJOR(
+			pd_get_rev(p->port, TCPCI_MSG_SOP));
+	} else {
+		r->sop_revision = 0;
+	}
+
 	r->sop_prime_revision =
 		pd_get_identity_discovery(p->port, TCPCI_MSG_SOP_PRIME) ==
 				PD_DISC_COMPLETE ?
diff --git a/common/usbc/usb_pe_drp_sm.c b/common/usbc/usb_pe_drp_sm.c
index 0714213cbd..a505a4922d 100644
--- a/common/usbc/usb_pe_drp_sm.c
+++ b/common/usbc/usb_pe_drp_sm.c
@@ -1459,7 +1459,7 @@ void pe_clear_ado(int port)
 	mutex_unlock(&pe[port].ado_lock);
 }
 
-struct rmdo pe_get_partner_rmdo(int port)
+struct rmdo pd_get_partner_rmdo(int port)
 {
 	return pe[port].partner_rmdo;
 }
diff --git a/include/ec_commands.h b/include/ec_commands.h
index 63a962d791..737bcbb77a 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -6941,6 +6941,18 @@ enum tcpc_cc_polarity {
 #define PD_STATUS_REV_GET_MINOR(r) ((r >> 8) & 0xF)
 
 /*
+ * Encode revision from partner RMDO
+ *
+ * Unlike the specification revision given in the PD header, specification and
+ * version information returned in the revision message data object (RMDO) is
+ * not offset.
+ */
+#define PD_STATUS_RMDO_REV_SET_MAJOR(r) (r << 12)
+#define PD_STATUS_RMDO_REV_SET_MINOR(r) (r << 8)
+#define PD_STATUS_RMDO_VER_SET_MAJOR(r) (r << 4)
+#define PD_STATUS_RMDO_VER_SET_MINOR(r) (r)
+
+/*
  * Decode helpers for Source and Sink Capability PDOs
  *
  * Note: The Power Delivery Specification should be considered the ultimate
@@ -7044,12 +7056,13 @@ struct ec_response_typec_status {
 	/*
 	 * BCD PD revisions for partners
 	 *
-	 * The format has the PD major reversion in the upper nibble, and PD
-	 * minor version in the next nibble.  Following two nibbles are
-	 * currently 0.
-	 * ex. PD 3.2 would map to 0x3200
+	 * The format has the PD major revision in the upper nibble, and the PD
+	 * minor revision in the next nibble. The following two nibbles hold the
+	 * major and minor specification version. If a partner does not support
+	 * the Revision message, only the major revision will be given.
+	 * ex. PD Revision 3.2 Version 1.9 would map to 0x3219
 	 *
-	 * PD major/minor will be 0 if no PD device is connected.
+	 * PD revision/version will be 0 if no PD device is connected.
 	 */
 	uint16_t sop_revision;
 	uint16_t sop_prime_revision;
diff --git a/include/usb_pd.h b/include/usb_pd.h
index 956fd3793d..cb31f986c7 100644
--- a/include/usb_pd.h
+++ b/include/usb_pd.h
@@ -3093,6 +3093,14 @@ uint32_t pd_get_requested_current(int port);
 bool pd_get_partner_usb_comm_capable(int port);
 
 /**
+ * Gets the port partner's RMDO from the PE state.
+ *
+ * @param port USB-C port number
+ * @return port partner's Revision Message Data Object (RMDO).
+ */
+struct rmdo pd_get_partner_rmdo(int port);
+
+/**
  * Return true if PD is in disconnect state
  *
  * @param port USB-C port number
diff --git a/include/usb_pe_sm.h b/include/usb_pe_sm.h
index a2e2a0f951..f8d20a0394 100644
--- a/include/usb_pe_sm.h
+++ b/include/usb_pe_sm.h
@@ -192,14 +192,6 @@ int pe_set_ado(int port, uint32_t data);
  */
 void pe_clear_ado(int port);
 
-/**
- * Gets port partner's RMDO from the PE state.
- *
- * @param port USB-C port number
- * @return port partner's Revision Message Data Object (RMDO).
- */
-struct rmdo pe_get_partner_rmdo(int port);
-
 #ifdef TEST_BUILD
 /**
  * Clears all internal port data, as we would on a detach event
diff --git a/zephyr/test/drivers/default/src/integration/usbc/usb_pd_rev3.c b/zephyr/test/drivers/default/src/integration/usbc/usb_pd_rev3.c
index 0ac22ae359..6a93e4704d 100644
--- a/zephyr/test/drivers/default/src/integration/usbc/usb_pd_rev3.c
+++ b/zephyr/test/drivers/default/src/integration/usbc/usb_pd_rev3.c
@@ -6,10 +6,12 @@
 #include "battery.h"
 #include "battery_smart.h"
 #include "chipset.h"
+#include "ec_commands.h"
 #include "emul/emul_isl923x.h"
 #include "emul/emul_smart_battery.h"
 #include "emul/tcpc/emul_tcpci_partner_src.h"
 #include "hooks.h"
+#include "host_command.h"
 #include "test/drivers/stubs.h"
 #include "test/drivers/test_state.h"
 #include "test/drivers/utils.h"
@@ -21,6 +23,8 @@
 #include <zephyr/sys/slist.h>
 #include <zephyr/ztest.h>
 
+#define TEST_PORT 0
+
 struct usb_attach_5v_3a_pd_source_rev3_fixture {
 	struct tcpci_partner_data source_5v_3a;
 	struct tcpci_src_emul_data src_ext;
@@ -172,6 +176,20 @@ ZTEST_F(usb_attach_5v_3a_pd_source_rev3, test_batt_cap_invalid)
 		"Invalid battery ref bit should be set");
 }
 
+ZTEST_F(usb_attach_5v_3a_pd_source_rev3, verify_typec_status_using_rmdo)
+{
+	struct ec_params_typec_status params = { .port = TEST_PORT };
+	struct ec_response_typec_status response;
+	struct host_cmd_handler_args args =
+		BUILD_HOST_COMMAND(EC_CMD_TYPEC_STATUS, 0, response, params);
+
+	/* Check that the revision response in EC_CMD_TYPEC_STATUS matches
+	 * bits 16-31 of the partner's RMDO
+	 */
+	zassert_ok(host_command_process(&args));
+	zassert_equal(response.sop_revision, fixture->source_5v_3a.rmdo >> 16);
+}
+
 ZTEST_F(usb_attach_5v_3a_pd_source_rev3, verify_alert_msg)
 {
 	zassert_equal(pd_broadcast_alert_msg(ADO_OTP_EVENT), EC_SUCCESS);