anx74xx: fix role bits for GOOD_CRC

There were 2 mistakes when setting the data/power roles for automatic
GOOD_CRC:
- the bit numbers for data role and power role were swapped.
- the function can only set and not reset the bits.

Try to simplify this code by:
- removing the duplicated name for register 0x9C (aka AUTO_GOODCRC_1)
- avoiding the multiple read/modify/write by using AUTO_GOODCRC_1 for
  the actual settings (and letting the enable bit always on)
  and GOOD_CRC_2 for enabling/disabling it, so we can do simple writes.
- answer only on SOP (not SOP' or SOP'').

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>

BRANCH=reef
BUG=b:35648282
TEST=On Snappy, connect a given power supply, record the USB PD traces
and see that the GOOD_CRC messages are still correct after the DR_SWAP.

Change-Id: I848b1dcbc0e06806649e64a9664f3fba21bdd448
Reviewed-on: https://chromium-review.googlesource.com/448040
Commit-Ready: Vincent Palatin <vpalatin@chromium.org>
Tested-by: Vincent Palatin <vpalatin@chromium.org>
Reviewed-by: S Wang <swang@analogix.corp-partner.google.com>
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
(cherry picked from commit c21ad5898e452bdd0646db3c1b6fbe4721034b8e)
Reviewed-on: https://chromium-review.googlesource.com/451678
Commit-Queue: Daisuke Nojiri <dnojiri@chromium.org>
Tested-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

2 files changed, 19 insertions, 37 deletions

diff --git a/driver/tcpm/anx74xx.c b/driver/tcpm/anx74xx.c
index 56728548c6..ad2c9514d0 100644
--- a/driver/tcpm/anx74xx.c
+++ b/driver/tcpm/anx74xx.c
@@ -50,27 +50,8 @@ static int anx74xx_tcpm_init(int port);
 
 static void anx74xx_tcpm_set_auto_good_crc(int port, int enable)
 {
-	int reg;
-
-	if (enable) {
-		/* Set default header for Good CRC auto reply */
-		tcpc_read(port, ANX74XX_REG_TX_MSG_HEADER, &reg);
-		reg |= (PD_REV20 << ANX74XX_REG_SPEC_REV_BIT_POS);
-		reg |= ANX74XX_REG_AUTO_GOODCRC_EN;
-		tcpc_write(port, ANX74XX_REG_TX_MSG_HEADER, reg);
-
-		reg = ANX74XX_REG_ENABLE_GOODCRC;
-		tcpc_write(port, ANX74XX_REG_TX_AUTO_GOODCRC_2, reg);
-		/* Set bit-0 if enable, reset bit-0 if disable */
-		tcpc_read(port, ANX74XX_REG_TX_AUTO_GOODCRC_1, &reg);
-		reg |= ANX74XX_REG_AUTO_GOODCRC_EN;
-	} else {
-		/* Clear bit-0 for disable */
-		tcpc_read(port, ANX74XX_REG_TX_AUTO_GOODCRC_1, &reg);
-		reg &= ~ANX74XX_REG_AUTO_GOODCRC_EN;
-		tcpc_write(port, ANX74XX_REG_TX_AUTO_GOODCRC_2, 0);
-	}
-	tcpc_write(port, ANX74XX_REG_TX_AUTO_GOODCRC_1, reg);
+	tcpc_write(port, ANX74XX_REG_TX_AUTO_GOODCRC_2,
+		   enable ? ANX74XX_REG_REPLY_SOP_EN : 0);
 }
 
 static void anx74xx_set_power_mode(int port, int mode)
@@ -646,14 +627,8 @@ static int anx74xx_tcpm_set_vconn(int port, int enable)
 
 static int anx74xx_tcpm_set_msg_header(int port, int power_role, int data_role)
 {
-	int rv = 0, reg;
-
-	rv |= tcpc_read(port, ANX74XX_REG_TX_MSG_HEADER, &reg);
-	reg |= ((!!power_role) << ANX74XX_REG_PWR_ROLE_BIT_POS);
-	reg |= ((!!data_role) << ANX74XX_REG_DATA_ROLE_BIT_POS);
-	rv |= tcpc_write(port, ANX74XX_REG_TX_MSG_HEADER, reg);
-
-	return rv;
+	return tcpc_write(port, ANX74XX_REG_TX_AUTO_GOODCRC_1,
+		  ANX74XX_REG_AUTO_GOODCRC_SET(!!data_role, !!power_role));
 }
 
 static int anx74xx_alert_status(int port, int *alert)
diff --git a/driver/tcpm/anx74xx.h b/driver/tcpm/anx74xx.h
index fcbd38fe41..127af4e0b5 100644
--- a/driver/tcpm/anx74xx.h
+++ b/driver/tcpm/anx74xx.h
@@ -63,10 +63,22 @@
 #define ANX74XX_REG_CC_PULL_RP		0x02
 
 
-#define ANX74XX_REG_TX_AUTO_GOODCRC_1	0x9c
 #define ANX74XX_REG_TX_AUTO_GOODCRC_2	0x94
-#define ANX74XX_REG_AUTO_GOODCRC_EN	0x01
-#define ANX74XX_REG_ENABLE_GOODCRC		0x38
+#define ANX74XX_REG_REPLY_SOP_EN        (1 << 3)
+#define ANX74XX_REG_REPLY_SOP_1_EN      (1 << 4)
+#define ANX74XX_REG_REPLY_SOP_2_EN      (1 << 5)
+
+#define ANX74XX_REG_TX_AUTO_GOODCRC_1	0x9c
+#define ANX74XX_REG_SPEC_REV_BIT_POS	(3)
+#define ANX74XX_REG_DATA_ROLE_BIT_POS	(2)
+#define ANX74XX_REG_PWR_ROLE_BIT_POS	(1)
+#define ANX74XX_REG_AUTO_GOODCRC_EN	(1 << 0)
+#define ANX74XX_REG_AUTO_GOODCRC_SET(drole, prole)    \
+	((PD_REV20 << ANX74XX_REG_SPEC_REV_BIT_POS) | \
+	((drole) << ANX74XX_REG_DATA_ROLE_BIT_POS)  | \
+	((prole) << ANX74XX_REG_PWR_ROLE_BIT_POS)   | \
+	ANX74XX_REG_AUTO_GOODCRC_EN)
+
 
 #define ANX74XX_REG_ANALOG_CTRL_0		0x41
 #define ANX74XX_REG_R_PIN_CABLE_DET		(1 << 7)
@@ -150,11 +162,6 @@
 #define ANX74XX_REG_HPD_DEFAULT		0x00
 #define ANX74XX_REG_HPD_OUT_DATA		0x10
 
-#define ANX74XX_REG_TX_MSG_HEADER		0x9c
-#define ANX74XX_REG_SPEC_REV_BIT_POS	(3)
-#define ANX74XX_REG_PWR_ROLE_BIT_POS	(2)
-#define ANX74XX_REG_DATA_ROLE_BIT_POS	(1)
-
 #define ANX74XX_REG_RECVD_MSG_INT 0x98
 #define ANX74XX_REG_CC_STATUS     0x99
 #define ANX74XX_REG_CTRL_FW  0x2E