From 9440fcfea914b8efc8ab5160326fcbeec78465e8 Mon Sep 17 00:00:00 2001 From: Namyoon Woo Date: Mon, 27 Jan 2020 13:57:40 -0800 Subject: control UART_EC RX/TX based on EC-CR50 communication status This patch allows EC-CR50 communication to enable EC UART only if EC is on and bitbang mode is disabled. EC UART shall be enabled even when CCD_CAP_GSC_TX_EC_RX is disabled, EC UART is ccdblocked, or servo is connected. EC-CR50 comm supporting boards are supposed to have H1 dominate EC UART TX line against servo. Servo detection, which is checked every second, shall be delayed during EC-CR50 communication because EC UART TX pin (GPIO_SERVO_DETECT) is used as an output. Servo state shall be held as it was. Once EC-CR50 communication is done, the servo detection will resume or CCD state gets updated based on what it used to be before EC-CR50 communication. BUG=chromium:1035706 BRANCH=cr50 TEST=manually tested on a reworked Helios. // CCD connection only > ccdstate AP: off AP UART: off EC: on Rdd: connected Servo: undetectable CCD EXT: enabled State flags: UARTEC+TX I2C SPI CCD ports blocked: (none) > ecrst pulse Pulsing EC reset EC_RST_L is deasserted // Servo connection only > ccdstate AP: off AP UART: off EC: on Rdd: disconnected Servo: connected CCD EXT: disabled State flags: I2C CCD ports blocked: (none) Change-Id: I02667bee004d237d846393a18f247970982c71b7 Signed-off-by: Namyoon Woo Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2023239 Reviewed-by: Vadim Bendebury --- board/cr50/rdd.c | 28 +++++++++++++++++++++++++--- board/cr50/servo_state.c | 12 ++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/board/cr50/rdd.c b/board/cr50/rdd.c index 9cc217129c..7a4b2de8d3 100644 --- a/board/cr50/rdd.c +++ b/board/cr50/rdd.c @@ -222,11 +222,18 @@ static void ccd_state_change_hook(void) /* Start out by figuring what flags we might want enabled */ - /* Enable EC/AP UART RX if that device is on */ + /* Enable AP UART RX if that device is on */ if (ap_uart_is_on()) flags_want |= CCD_ENABLE_UART_AP; - if (ec_is_rx_allowed()) - flags_want |= CCD_ENABLE_UART_EC; + /* + * Enable EC UART RX. + * Checking that EC is off (ec_is_rx_allowed()) will be done in the end, + * and CCD_ENABLE_UART_EC will be cleared if so. This is to guarantee + * that EC UART RX won't be enabled if EC is off even with any codes + * overriding this flag. We also intend to keep ec_is_rx_allowed() + * called once. + */ + flags_want |= CCD_ENABLE_UART_EC; #ifdef CONFIG_UART_BITBANG if (uart_bitbang_is_wanted()) @@ -289,6 +296,21 @@ static void ccd_state_change_hook(void) if (ccd_block & CCD_BLOCK_EC_UART) flags_want &= ~CCD_ENABLE_UART_EC; + /* + * EC UART flags are cleared if ccd ext is not detected or if ccd block + * EC UART is enabled. EC-CR50 comm trumps both of those conditions. + * Re-enable the EC UART flags if EC-CR50 comm is enabled. + */ + if (ec_comm_is_uart_in_packet_mode(UART_EC)) + flags_want |= (CCD_ENABLE_UART_EC | CCD_ENABLE_UART_EC_TX); + + /* + * Disable EC UART RX if that device is off, otherwise there will be + * an UART interrupt storm. + */ + if (!ec_is_rx_allowed()) + flags_want &= ~CCD_ENABLE_UART_EC; + /* UARTs are either RX-only or RX+TX, so no RX implies no TX */ if (!(flags_want & CCD_ENABLE_UART_AP)) flags_want &= ~CCD_ENABLE_UART_AP_TX; diff --git a/board/cr50/servo_state.c b/board/cr50/servo_state.c index f6ded01dd0..5c05d2bb77 100644 --- a/board/cr50/servo_state.c +++ b/board/cr50/servo_state.c @@ -146,6 +146,18 @@ static void servo_detect(void) if (state == DEVICE_STATE_IGNORED) return; + /* + * During EC-CR50 communication, do not change servo state because + * GPIO_DETECT_SERVO (DIOB5) is not available. Return now, and + * let it try to detect in the next second. + * + * Note: Though servo is not detectable, we do not want to change + * servo_state as UNDETECTABLE, otherwise "servo_is_connected()" + * might return false while servo is connected. + */ + if (ec_comm_is_uart_in_packet_mode(UART_EC)) + return; + /* If we're driving EC UART TX, we can't detect servo */ if (!servo_detectable()) { /* We're driving one port; might as well drive them all */ -- cgit v1.2.1