From 949f80604eec3c4f6d4a6fcfb2970461b221f00e Mon Sep 17 00:00:00 2001 From: Denis Brockus Date: Fri, 31 Jan 2020 07:37:05 -0700 Subject: 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 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2033304 Reviewed-by: Edward Hill Reviewed-by: Jett Rink --- baseboard/zork/baseboard.c | 90 +++++++++++++++++++++++++--------------------- driver/retimer/ps8818.c | 43 +++++++++++++++------- driver/retimer/ps8818.h | 8 +++-- 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 */ -- cgit v1.2.1