cr50: Have INAs and I2Cm enabled when rdd is attached

The INAs are only used for development and testing
purposes. Therefore, the 3.3V rail to the INAs is off by default and
the I2Cm module is not enabled. Enabling INA power and connecting the
I2Cm module was done at the beginning of each USB to I2C request. The
problem with this approach is that INA measurments didn't always
succeed due to not enough time for the INAs to initialize.

Rather than add some arbitrary delay, it is better to tie the INAs to
when rdd is attached/detached. It is only when rdd is attached that
the INAs will be accessed, so there is no need to enable/disable for
each individual I2C transaction.

This CL ties the enabling/disabling of the INA and I2Cm module to the
rdd state. This change makes the previous use of
usb_i2c_board_enable() and usb_i2c_board_disable() obslete.

BRANCH=none
BUG=chrome-os-partner:62375
TEST=manual
Connect servo with suzyq connected:
sudo servod -p 0x5014 -b eve -c eve_r0_inas.xml
Then execute single INA reads dut-control pp3300_dx_edp_mv and verify
that it returns meaningful numbers. Without this CL single reads via
dut-control would always return 0.

Change-Id: I799552bfd0701efd1828a0d720ac2a6cedee5ca1
Signed-off-by: Scott <scollyer@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/436864
Commit-Ready: Scott Collyer <scollyer@chromium.org>
Tested-by: Scott Collyer <scollyer@chromium.org>
Reviewed-by: Mary Ruthven <mruthven@chromium.org>

7 files changed, 21 insertions, 34 deletions

diff --git a/board/cr50/board.c b/board/cr50/board.c
index d47cb41729..01d9a0d5a9 100644
--- a/board/cr50/board.c
+++ b/board/cr50/board.c
@@ -776,7 +776,7 @@ static void servo_attached(void)
 	uartn_tx_disconnect(UART_EC);
 
 	/* Disconnect i2cm interface to ina */
-	usb_i2c_board_disable(0);
+	usb_i2c_board_disable();
 }
 
 void device_state_on(enum gpio_signal signal)
diff --git a/board/cr50/rdd.c b/board/cr50/rdd.c
index df0e097e81..da93b1c401 100644
--- a/board/cr50/rdd.c
+++ b/board/cr50/rdd.c
@@ -121,6 +121,9 @@ void rdd_attached(void)
 	enable_usb_wakeup = 1;
 
 	uartn_tx_connect(UART_AP);
+
+	/* Turn on 3.3V rail used for INAs and initialize I2CM module */
+	usb_i2c_board_enable();
 }
 
 void rdd_detached(void)
@@ -140,6 +143,9 @@ void rdd_detached(void)
 
 	/* Disable CCD */
 	ccd_set_mode(CCD_MODE_DISABLED);
+
+	/* Turn off 3.3V rail to INAs and disconnect I2CM module */
+	usb_i2c_board_disable();
 }
 
 void ccd_phy_init(int enable_ccd)
@@ -220,7 +226,7 @@ static int command_ccd(int argc, char **argv)
 			if (val)
 				usb_i2c_board_enable();
 			else
-				usb_i2c_board_disable(0);
+				usb_i2c_board_disable();
 		} else if (!strcasecmp("keepalive", argv[1])) {
 			if (val) {
 				/* Make sure ccd is enabled */
diff --git a/board/cr50/usb_i2c.c b/board/cr50/usb_i2c.c
index 8ba90c5f63..786a886c7f 100644
--- a/board/cr50/usb_i2c.c
+++ b/board/cr50/usb_i2c.c
@@ -35,7 +35,6 @@ static void ina_disconnect(void)
 	/* Disable power to INA chips */
 	gpio_set_level(GPIO_EN_PP3300_INA_L, 1);
 }
-DECLARE_DEFERRED(ina_disconnect);
 
 static void ina_connect(void)
 {
@@ -65,29 +64,24 @@ static void ina_connect(void)
 	i2cm_init();
 }
 
-void usb_i2c_board_disable(int debounce)
+void usb_i2c_board_disable(void)
 {
 	if (!i2c_enabled())
 		return;
 
-	/*
-	 * Wait to disable i2c in case we are doing a bunch of i2c transactions
-	 * in a row.
-	 */
-	hook_call_deferred(&ina_disconnect_data, debounce ? 1 * SECOND : 0);
+	ina_disconnect();
 }
 
 int usb_i2c_board_enable(void)
 {
-	if (device_get_state(DEVICE_SERVO) != DEVICE_STATE_OFF) {
+	if (device_get_state(DEVICE_SERVO) == DEVICE_STATE_ON) {
 		CPRINTS("Servo is attached I2C cannot be enabled");
-		usb_i2c_board_disable(0);
+		usb_i2c_board_disable();
 		return EC_ERROR_BUSY;
 	}
 
-	hook_call_deferred(&ina_disconnect_data, -1);
-
 	if (!i2c_enabled())
 		ina_connect();
+
 	return EC_SUCCESS;
 }
diff --git a/board/servo_micro/board.c b/board/servo_micro/board.c
index 853de87746..478501fda9 100644
--- a/board/servo_micro/board.c
+++ b/board/servo_micro/board.c
@@ -199,9 +199,6 @@ const struct i2c_port_t i2c_ports[] = {
 };
 const unsigned int i2c_ports_used = ARRAY_SIZE(i2c_ports);
 
-int usb_i2c_board_enable(void) {return EC_SUCCESS; }
-void usb_i2c_board_disable(int debounce) {}
-
 
 /******************************************************************************
  * Support firmware upgrade over USB. We can update whichever section is not
diff --git a/board/servo_v4/board.c b/board/servo_v4/board.c
index 99a0fa1500..b12126c7d8 100644
--- a/board/servo_v4/board.c
+++ b/board/servo_v4/board.c
@@ -237,9 +237,6 @@ const struct i2c_port_t i2c_ports[] = {
 };
 const unsigned int i2c_ports_used = ARRAY_SIZE(i2c_ports);
 
-int usb_i2c_board_enable(void) {return EC_SUCCESS; }
-void usb_i2c_board_disable(int debounce) {}
-
 
 /******************************************************************************
  * Support firmware upgrade over USB. We can update whichever section is not
diff --git a/common/usb_i2c.c b/common/usb_i2c.c
index 1795a26498..4c8350e910 100644
--- a/common/usb_i2c.c
+++ b/common/usb_i2c.c
@@ -63,7 +63,6 @@ void usb_i2c_deferred(struct usb_i2c_config const *config)
 	int write_count     = (config->buffer[1] >> 0) & 0xff;
 	int read_count      = (config->buffer[1] >> 8) & 0xff;
 	int port;
-	int rv;
 
 	config->buffer[0] = 0;
 	config->buffer[1] = 0;
@@ -79,19 +78,13 @@ void usb_i2c_deferred(struct usb_i2c_config const *config)
 	} else if (portindex >= i2c_ports_used) {
 		config->buffer[0] = USB_I2C_PORT_INVALID;
 	} else {
-		rv = usb_i2c_board_enable();
-		if (rv) {
-			config->buffer[0] = usb_i2c_map_error(rv);
-		} else {
-			port = i2c_ports[portindex].port;
-			config->buffer[0] = usb_i2c_map_error(
-				i2c_xfer(port, slave_addr,
-					(uint8_t *)(config->buffer + 2),
-					write_count,
-					(uint8_t *)(config->buffer + 2),
-					read_count, I2C_XFER_SINGLE));
-			usb_i2c_board_disable(1);
-		}
+		port = i2c_ports[portindex].port;
+		config->buffer[0] = usb_i2c_map_error(
+			i2c_xfer(port, slave_addr,
+				 (uint8_t *)(config->buffer + 2),
+				 write_count,
+				 (uint8_t *)(config->buffer + 2),
+				 read_count, I2C_XFER_SINGLE));
 	}
 
 	usb_i2c_write_packet(config, read_count + 4);
diff --git a/include/usb_i2c.h b/include/usb_i2c.h
index 163cdfb98a..a1fe5f10db 100644
--- a/include/usb_i2c.h
+++ b/include/usb_i2c.h
@@ -149,5 +149,5 @@ void usb_i2c_deferred(struct usb_i2c_config const *config);
  * specific operations required to enable or disable access to the I2C device.
  */
 int usb_i2c_board_enable(void);
-void usb_i2c_board_disable(int debounce);
+void usb_i2c_board_disable(void);
 #endif  /* __CROS_USB_I2C_H */