summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorVadim Bendebury <vbendeb@chromium.org>2018-04-17 17:39:09 -0700
committerchrome-bot <chrome-bot@chromium.org>2018-05-22 15:54:10 -0700
commit32b1e3add72159df481ea5e3d86b581ef07caaa3 (patch)
treea47b74486a213c3f42fd43905b5a200ad03847d2
parent0a602bd7b4be36aa767b6d4b61dd0445c09d3292 (diff)
downloadchrome-ec-32b1e3add72159df481ea5e3d86b581ef07caaa3.tar.gz
g: speed up CCD UART processing
AP and EC consoles may generate a lot of bursty traffic, and cr50 UART console to USB processing is very slow: when characters become available, a hooks task callback is invoked, which retrieves received characters one at a time and queues them up to the appropriate USB transmit queue. This patch speeds up things as follows: - increases the seize of USB transmit queues for AP and EC console channels to 512 bytes. Experiments supported by code instrumentation has shown that even this is not enough to avoid underruns, but this is a good compromise between memory use and performance, these sizes could be revisited later, - raises UART RX interrupt priority from level 1 to 0 - moving bytes from UART TX FIFO to USB queue happens on the interrupt context when UART TX interrupt is asserted - as many characters as possible are read from the UART first, before queuing function is called, and the entire received batch is passed to the queuing function. It has to be mentioned here that presently batch processing is not necessarily much more efficient, because queuing function becomes more complicated when multiple objects are passed to it, this will have to be dealt with in a separate patch. There is still a lot of room for improvement: - functions used to queue up data are very generic, dedicated code could help a lot. - UART drivers should have methods for collecting all bytes available in receive FIFO in one invocation, - USB side of things (dequeuing data and passing it to the controller. BRANCH=cr50, cr50mp BUG=b:38448364 TEST=ran 'chargen' application on both AP and EC to flood the console channels and observed the flow of characters on the host site, it is pretty smooth with occasional hiccups, especially when TPM is active, before this patch it was impossible to have both stream up, both were garbled. - Verified that new account can be created and user logged in on restarts while chargen is running, i.e. TPM task gets enough processing bandwidth. - When EC is reset, there seem to be no lost characters on the console (it used to cause some garbled console output before this patch). The below output was collected on Coral: > reboot Rebooting! --- UART initialized after reboot --- [Reset cause: soft] [Image: RO, coral_v1.1.8363+2cc945d5a 2018-05-15 17:41:57 ... [0.003605 init buttons] [0.003826 Inits done] [0.004094 tablet mode disabled ] [0.008272 found batt:SMP] [0.022278 SW 0x01] [0.042247 hash start 0x00040000 0x00021994] [0.045823 Battery FET: reg 0x0018 mask 0x0018 disc 0x0000] [0.071136 kblight registered] [0.071544 PB init-on] [0.071818 USB charge p0 m0] [0.073670 ID/SKU ADC 4 = 1309 mV] [0.075630 ID/SKU ADC 3 = 852 mV] [0.076077 SKU ID: 71] [0.076335 Motion Sensor Count = 3] [0.083594 PD comm enabled] ... - did not test bitbang programming mode, it is in line for reworking for speeding up as well. Change-Id: Ic9f3972f585dd1976169965c2a2422253aeac87a Signed-off-by: Vadim Bendebury <vbendeb@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1016037 Reviewed-by: Randall Spangler <rspangler@chromium.org> Reviewed-by: Mary Ruthven <mruthven@chromium.org>
-rw-r--r--board/cr50/board.c13
-rw-r--r--chip/g/usart.c33
-rw-r--r--chip/g/usart.h26
3 files changed, 42 insertions, 30 deletions
diff --git a/board/cr50/board.c b/board/cr50/board.c
index ba09d5ad32..ab976dc29b 100644
--- a/board/cr50/board.c
+++ b/board/cr50/board.c
@@ -32,6 +32,7 @@
#include "trng.h"
#include "uart_bitbang.h"
#include "uartn.h"
+#include "usart.h"
#include "usb_descriptor.h"
#include "usb_hid.h"
#include "usb_i2c.h"
@@ -103,12 +104,18 @@ struct uart_bitbang_properties bitbang_config = {
.rx_pinmux_regval = GC_PINMUX_GPIO1_GPIO4_SEL,
};
-extern struct deferred_data ec_uart_deferred__data;
void ec_tx_cr50_rx(enum gpio_signal signal)
{
uart_bitbang_receive_char(UART_EC);
- /* Let the USART module know that there's new bits to consume. */
- hook_call_deferred(&ec_uart_deferred__data, 0);
+ /*
+ * Let the USART module know that there's new bits to consume.
+ *
+ * When programming the EC in bitbang mode the rest of the system is
+ * shut down, there not much else to do, so this could be processed
+ * directly on interrupt context the same way it is done with EC
+ * console output.
+ */
+ send_data_to_usb(&ec_uart);
}
const char *device_state_names[] = {
diff --git a/chip/g/usart.c b/chip/g/usart.c
index 5236acf202..6580ca9eb0 100644
--- a/chip/g/usart.c
+++ b/chip/g/usart.c
@@ -15,6 +15,11 @@
#define USE_UART_INTERRUPTS (!(defined(CONFIG_CUSTOMIZED_RO) && \
defined(SECTION_IS_RO)))
#define QUEUE_SIZE 64
+/*
+ * Want to be able to accumulate larger amounts of data while USB is
+ * momentarily stalled for whatever reason.
+ */
+#define QUEUE_SIZE_UART_RX 512
#ifdef CONFIG_STREAM_SIGNATURE
/*
@@ -57,7 +62,8 @@ SIGNER_CONFIG(sig, stream_uart, sig_to_usb, ap_uart_output);
#else /* Not CONFIG_STREAM_SIGNATURE */
static struct queue const ap_uart_output =
- QUEUE_DIRECT(QUEUE_SIZE, uint8_t, ap_uart.producer, ap_usb.consumer);
+ QUEUE_DIRECT(QUEUE_SIZE_UART_RX, uint8_t,
+ ap_uart.producer, ap_usb.consumer);
#endif
static struct queue const ap_usb_to_uart =
@@ -100,7 +106,8 @@ struct usb_stream_config const ec_usb;
struct usart_config const ec_uart;
static struct queue const ec_uart_to_usb =
- QUEUE_DIRECT(QUEUE_SIZE, uint8_t, ec_uart.producer, ec_usb.consumer);
+ QUEUE_DIRECT(QUEUE_SIZE_UART_RX, uint8_t,
+ ec_uart.producer, ec_usb.consumer);
static struct queue const ec_usb_to_uart =
QUEUE_DIRECT(QUEUE_SIZE, uint8_t, ec_usb.producer, ec_uart.consumer);
@@ -135,14 +142,26 @@ void get_data_from_usb(struct usart_config const *config)
void send_data_to_usb(struct usart_config const *config)
{
+ /*
+ * UART RX FIFO is 32 bytes in size, let's have little extra room so
+ * that we could catch up if we are draining the FIFO while the chip
+ * keeps receiving.
+ */
+ uint8_t buffer[50];
+ uint32_t i;
+ uint32_t room;
struct queue const *uart_in = config->producer.queue;
+ int uart = config->uart;
+
+
+ i = 0;
+ room = MIN(sizeof(buffer), queue_space(uart_in));
- /* Copy input from buffer until RX fifo empty or the queue is full */
- while (uartn_rx_available(config->uart) && queue_space(uart_in)) {
- int c = uartn_read_char(config->uart);
+ while ((i < room) && uartn_rx_available(uart))
+ buffer[i++] = uartn_read_char(uart);
- QUEUE_ADD_UNITS(uart_in, &c, 1);
- }
+ if (i)
+ QUEUE_ADD_UNITS(uart_in, buffer, i);
}
static void uart_read(struct producer const *producer, size_t count)
diff --git a/chip/g/usart.h b/chip/g/usart.h
index 0be7a920bb..dacd7d1850 100644
--- a/chip/g/usart.h
+++ b/chip/g/usart.h
@@ -12,24 +12,13 @@
#ifndef __CROS_FORWARD_UART_H
#define __CROS_FORWARD_UART_H
-#ifdef CONFIG_STREAM_SIGNATURE
-/*
- * When configured for signing over streaming data, call the consumer handler
- * directly to help avoid incoming uart overruns.
- * Note this will run under interrupt handler so consumer beware.
- */
#define CONFIGURE_INTERRUPTS__rx_int(NAME) send_data_to_usb(&NAME)
-#else
-#define CONFIGURE_INTERRUPTS__rx_int(NAME) hook_call_deferred(NAME.deferred, 0)
-#endif
struct usart_config {
int uart;
struct producer const producer;
struct consumer const consumer;
-
- const struct deferred_data *deferred;
};
extern struct consumer_ops const uart_consumer_ops;
@@ -39,7 +28,7 @@ extern struct producer_ops const uart_producer_ops;
TXINT) \
void CONCAT2(NAME, _rx_int_)(void); \
void CONCAT2(NAME, _tx_int_)(void); \
- DECLARE_IRQ(RXINT, CONCAT2(NAME, _rx_int_), 1); \
+ DECLARE_IRQ(RXINT, CONCAT2(NAME, _rx_int_), 0); \
DECLARE_IRQ(TXINT, CONCAT2(NAME, _tx_int_), 1); \
void CONCAT2(NAME, _tx_int_)(void) \
{ \
@@ -63,8 +52,6 @@ extern struct producer_ops const uart_producer_ops;
UART, \
RX_QUEUE, \
TX_QUEUE) \
- static void CONCAT2(NAME, _deferred_)(void); \
- DECLARE_DEFERRED(CONCAT2(NAME, _deferred_)); \
struct usart_config const NAME = { \
.uart = UART, \
.consumer = { \
@@ -75,12 +62,7 @@ extern struct producer_ops const uart_producer_ops;
.queue = &RX_QUEUE, \
.ops = &uart_producer_ops, \
}, \
- .deferred = &CONCAT2(NAME, _deferred__data), \
- }; \
- static void CONCAT2(NAME, _deferred_)(void) \
- { \
- send_data_to_usb(&NAME); \
- } \
+ }
/* Read data from UART and add it to the producer queue */
@@ -88,4 +70,8 @@ void send_data_to_usb(struct usart_config const *config);
/* Read data from the consumer queue and send it to the UART */
void get_data_from_usb(struct usart_config const *config);
+
+/* Helper for UART bitbang mode. */
+extern struct usart_config const ec_uart;
+
#endif /* __CROS_FORWARD_UART_H */