PS8818: use read/modify/write for regs with reserved bits

PS88xx has a tendency to document undocumented register
bits as reserved.  Some of these are reserved and others
should not be reset to 0 and should remain the value
they were previously.  The gain control appears to be
of the latter type on the PS8818

BUG=b:146394157
BRANCH=none
TEST=verify USB-C1 DP and USB connections

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

3 files changed, 86 insertions, 55 deletions

diff --git a/baseboard/zork/baseboard.c b/baseboard/zork/baseboard.c
index 8f2a324185..8927b7a2a7 100644
--- a/baseboard/zork/baseboard.c
+++ b/baseboard/zork/baseboard.c
@@ -535,59 +535,67 @@ static int ps8818_tune_mux(int port, mux_state_t mux_state)
 	/* USB specific config */
 	if (mux_state & USB_PD_MUX_USB_ENABLED) {
 		/* Boost the USB gain */
-		rv = ps8818_i2c_write(port,
-				      PS8818_REG_PAGE1,
-				      PS8818_REG1_APTX1EQ_10G_LEVEL,
-				      PS8818_EQ_LEVEL_UP_21DB);
+		rv = ps8818_i2c_field_update8(port,
+					PS8818_REG_PAGE1,
+					PS8818_REG1_APTX1EQ_10G_LEVEL,
+					PS8818_EQ_LEVEL_UP_MASK,
+					PS8818_EQ_LEVEL_UP_21DB);
 		if (rv)
 			return rv;
 
-		rv = ps8818_i2c_write(port,
-				      PS8818_REG_PAGE1,
-				      PS8818_REG1_APTX2EQ_10G_LEVEL,
-				      PS8818_EQ_LEVEL_UP_21DB);
+		rv = ps8818_i2c_field_update8(port,
+					PS8818_REG_PAGE1,
+					PS8818_REG1_APTX2EQ_10G_LEVEL,
+					PS8818_EQ_LEVEL_UP_MASK,
+					PS8818_EQ_LEVEL_UP_21DB);
 		if (rv)
 			return rv;
 
-		rv = ps8818_i2c_write(port,
-				      PS8818_REG_PAGE1,
-				      PS8818_REG1_APTX1EQ_5G_LEVEL,
-				      PS8818_EQ_LEVEL_UP_21DB);
+		rv = ps8818_i2c_field_update8(port,
+					PS8818_REG_PAGE1,
+					PS8818_REG1_APTX1EQ_5G_LEVEL,
+					PS8818_EQ_LEVEL_UP_MASK,
+					PS8818_EQ_LEVEL_UP_21DB);
 		if (rv)
 			return rv;
 
-		rv = ps8818_i2c_write(port,
-				      PS8818_REG_PAGE1,
-				      PS8818_REG1_APTX2EQ_5G_LEVEL,
-				      PS8818_EQ_LEVEL_UP_21DB);
+		rv = ps8818_i2c_field_update8(port,
+					PS8818_REG_PAGE1,
+					PS8818_REG1_APTX2EQ_5G_LEVEL,
+					PS8818_EQ_LEVEL_UP_MASK,
+					PS8818_EQ_LEVEL_UP_21DB);
 		if (rv)
 			return rv;
 
-		rv = ps8818_i2c_write(port,
-				      PS8818_REG_PAGE1,
-				      PS8818_REG1_CRX1EQ_10G_LEVEL,
-				      PS8818_EQ_LEVEL_UP_21DB);
+		rv = ps8818_i2c_field_update8(port,
+					PS8818_REG_PAGE1,
+					PS8818_REG1_CRX1EQ_10G_LEVEL,
+					PS8818_EQ_LEVEL_UP_MASK,
+					PS8818_EQ_LEVEL_UP_21DB);
 		if (rv)
 			return rv;
 
-		rv = ps8818_i2c_write(port,
-				      PS8818_REG_PAGE1,
-				      PS8818_REG1_CRX2EQ_10G_LEVEL,
-				      PS8818_EQ_LEVEL_UP_21DB);
+		rv = ps8818_i2c_field_update8(port,
+					PS8818_REG_PAGE1,
+					PS8818_REG1_CRX2EQ_10G_LEVEL,
+					PS8818_EQ_LEVEL_UP_MASK,
+					PS8818_EQ_LEVEL_UP_21DB);
 		if (rv)
 			return rv;
 
-		rv = ps8818_i2c_write(port,
-				      PS8818_REG_PAGE1,
-				      PS8818_REG1_CRX1EQ_5G_LEVEL,
-				      PS8818_EQ_LEVEL_UP_21DB);
+		rv = ps8818_i2c_field_update8(port,
+					PS8818_REG_PAGE1,
+					PS8818_REG1_CRX1EQ_5G_LEVEL,
+					PS8818_EQ_LEVEL_UP_MASK,
+					PS8818_EQ_LEVEL_UP_21DB);
 		if (rv)
 			return rv;
 
-		rv = ps8818_i2c_write(port,
-				      PS8818_REG_PAGE1,
-				      PS8818_REG1_CRX2EQ_5G_LEVEL,
-				      PS8818_EQ_LEVEL_UP_21DB);
+		rv = ps8818_i2c_field_update8(port,
+					PS8818_REG_PAGE1,
+					PS8818_REG1_CRX2EQ_5G_LEVEL,
+					PS8818_EQ_LEVEL_UP_MASK,
+					PS8818_EQ_LEVEL_UP_21DB);
 		if (rv)
 			return rv;
 	}
@@ -597,10 +605,11 @@ static int ps8818_tune_mux(int port, mux_state_t mux_state)
 		int val;
 
 		/* Boost the DP gain */
-		rv = ps8818_i2c_write(port,
-				      PS8818_REG_PAGE1,
-				      PS8818_REG1_DPEQ_LEVEL,
-				      PS8818_DPEQ_LEVEL_UP_21DB);
+		rv = ps8818_i2c_field_update8(port,
+					PS8818_REG_PAGE1,
+					PS8818_REG1_DPEQ_LEVEL,
+					PS8818_DPEQ_LEVEL_UP_MASK,
+					PS8818_DPEQ_LEVEL_UP_21DB);
 		if (rv)
 			return rv;
 
@@ -609,10 +618,11 @@ static int ps8818_tune_mux(int port, mux_state_t mux_state)
 				? PS8818_LANE_COUNT_SET_2_LANE
 				: PS8818_LANE_COUNT_SET_4_LANE;
 
-		rv = ps8818_i2c_write(port,
-				      PS8818_REG_PAGE2,
-				      PS8818_REG2_LANE_COUNT_SET,
-				      val);
+		rv = ps8818_i2c_field_update8(port,
+					PS8818_REG_PAGE2,
+					PS8818_REG2_LANE_COUNT_SET,
+					PS8818_LANE_COUNT_SET_MASK,
+					val);
 		if (rv)
 			return rv;
 	}
diff --git a/driver/retimer/ps8818.c b/driver/retimer/ps8818.c
index 6059f7f0d4..7facba04d2 100644
--- a/driver/retimer/ps8818.c
+++ b/driver/retimer/ps8818.c
@@ -46,6 +46,22 @@ int ps8818_i2c_write(int port, int page, int offset, int data)
 			  offset, data);
 }
 
+int ps8818_i2c_field_update8(int port, int page, int offset,
+			     uint8_t field_mask, uint8_t set_value)
+{
+	if (PS8818_DEBUG)
+		ccprintf("%s(%d:0x%02X, 0x%02X, 0x%02X, 0x%02X)\n", __func__,
+			 usb_retimers[port].i2c_port,
+			 usb_retimers[port].i2c_addr_flags + page,
+			 offset, field_mask, set_value);
+
+	return i2c_field_update8(usb_retimers[port].i2c_port,
+				 usb_retimers[port].i2c_addr_flags + page,
+				 offset,
+				 field_mask,
+				 set_value);
+}
+
 int ps8818_detect(int port)
 {
 	int rv = EC_ERROR_NOT_POWERED;
@@ -84,10 +100,11 @@ static int ps8818_set_mux(int port, mux_state_t mux_state)
 	if (mux_state & USB_PD_MUX_DP_ENABLED)
 		val |= PS8818_MODE_DP_ENABLE;
 
-	rv = ps8818_i2c_write(port,
-			      PS8818_REG_PAGE0,
-			      PS8818_REG0_MODE,
-			      val);
+	rv = ps8818_i2c_field_update8(port,
+				PS8818_REG_PAGE0,
+				PS8818_REG0_MODE,
+				PS8818_MODE_NON_RESERVED_MASK,
+				val);
 	if (rv)
 		return rv;
 
@@ -96,10 +113,11 @@ static int ps8818_set_mux(int port, mux_state_t mux_state)
 	if (mux_state & USB_PD_MUX_POLARITY_INVERTED)
 		val |= PS8818_FLIP_CONFIG;
 
-	rv = ps8818_i2c_write(port,
-			      PS8818_REG_PAGE0,
-			      PS8818_REG0_FLIP,
-			      val);
+	rv = ps8818_i2c_field_update8(port,
+				PS8818_REG_PAGE0,
+				PS8818_REG0_FLIP,
+				PS8818_FLIP_NON_RESERVED_MASK,
+				val);
 	if (rv)
 		return rv;
 
@@ -108,10 +126,11 @@ static int ps8818_set_mux(int port, mux_state_t mux_state)
 	if (mux_state & USB_PD_MUX_DP_ENABLED)
 		val |= PS8818_DPHPD_PLUGGED;
 
-	rv = ps8818_i2c_write(port,
-			      PS8818_REG_PAGE0,
-			      PS8818_REG0_DPHPD_CONFIG,
-			      val);
+	rv = ps8818_i2c_field_update8(port,
+				PS8818_REG_PAGE0,
+				PS8818_REG0_DPHPD_CONFIG,
+				PS8818_DPHPD_NON_RESERVED_MASK,
+				val);
 	if (rv)
 		return rv;
 
diff --git a/driver/retimer/ps8818.h b/driver/retimer/ps8818.h
index 0b5c3dfe83..ff11ff930e 100644
--- a/driver/retimer/ps8818.h
+++ b/driver/retimer/ps8818.h
@@ -17,14 +17,17 @@
 
 #define PS8818_REG0_FLIP		0x00
 #define PS8818_FLIP_CONFIG			BIT(7)
+#define PS8818_FLIP_NON_RESERVED_MASK		0xE0
 
 #define PS8818_REG0_MODE		0x01
 #define PS8818_MODE_DP_ENABLE			BIT(7)
 #define PS8818_MODE_USB_ENABLE			BIT(6)
+#define PS8818_MODE_NON_RESERVED_MASK		0xC0
 
 #define PS8818_REG0_DPHPD_CONFIG	0x02
 #define PS8818_DPHPD_CONFIG_INHPD_DISABLE	BIT(7)
 #define PS8818_DPHPD_PLUGGED			BIT(6)
+#define PS8818_DPHPD_NON_RESERVED_MASK		0xFC
 
 /*
  * PAGE 1 Register Definitions
@@ -64,9 +67,6 @@
 #define PS8818_DPEQ_LEVEL_UP_21DB		(9 << 3)
 #define PS8818_DPEQ_LEVEL_UP_MASK		(0x0F << 3)
 
-#define PS8818_REG1_LINK_TRAINING	0xB7
-#define PS8818_LINK_TRAINING_DISABLE		BIT(4)
-
 /*
  * PAGE 2 Register Definitions
  */
@@ -96,5 +96,7 @@ int ps8818_detect(int port);
 
 int ps8818_i2c_read(int port, int page, int offset, int *data);
 int ps8818_i2c_write(int port, int page, int offset, int data);
+int ps8818_i2c_field_update8(int port, int page, int offset,
+			     uint8_t field_mask, uint8_t set_value);
 
 #endif /* __CROS_EC_USB_RETIMER_PS8818_H */