The UART DM controller supports different channel data packing modes,
either the 4-character packing mode (where 32-bit are read/written at once)
or the single-character mode (where only a single character is read/written
at a time). The 4-character mode can be more efficient, but the
single-character mode is much easier to implement.
At the moment, serial_msm uses the 4-character mode. Since the
dm_serial_ops operate on one character at the time, the code goes through
quite some hoops in order to break this down to single characters. This
code is prone to race conditions (e.g. priv->chars_cnt is read from the
registers, then a command is issued, what if another char came in
inbetween?). It also seems to cause another subtle issue with autoboot:
Unlike the previous autoboot failures that happened when UART was
disconnected, this problem occurs when UART is connected and open in a
terminal: For EFI boot, the console size is queried in efi_console.c
query_console_serial() by sending an ANSI escape code via UART. For some
reason, with the current driver we get yet another 0x00 byte (UART break
event?) when reading the reply from serial input. Because of that, reading
the console size fails in efi_console.c, the actual reply remains in the
UART buffer, and later the boot flow aborts because it detects input after
printing a prompt.
Rather than trying to fix the issue in the current complicated approach,
switch the driver to use the single-character mode. This is simple and
straightforward to implement without race conditions:
- We write one character at a time to UARTDM_TF, as long as the TX FIFO
has space available (TX_READY). To flush the console before starting
Linux, we wait for TX_EMPTY.
- We read one character at a time from UARTDM_RF and strip off the
additional error information (assuming there is something in the
RX FIFO, as indicated by RX_READY).
In this mode, querying the serial console size works and autoboot is no
longer interrupted. The overall code is also much shorter.
Signed-off-by: Stephan Gerhold <stephan.gerh...@linaro.org>
---
drivers/serial/serial_msm.c | 102 +++++++-------------------------------------
1 file changed, 16 insertions(+), 86 deletions(-)
diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
index
2c08a84b02773663f3c3a11eddaaa7a8bab2b4a1..18d15b7b15b3aeb2e8fd1da1aa49967d55c943ab
100644
--- a/drivers/serial/serial_msm.c
+++ b/drivers/serial/serial_msm.c
@@ -21,13 +21,9 @@
/* Serial registers - this driver works in uartdm mode*/
-#define UARTDM_DMRX 0x34 /* Max RX transfer length */
-#define UARTDM_DMEN 0x3C /* DMA/data-packing mode */
-#define UARTDM_NCF_TX 0x40 /* Number of chars to TX */
+#define UARTDM_DMEN 0x3C /* DMA/data-packing mode */
+#define UARTDM_DMEN_TXRX_SC_ENABLE (BIT(4) | BIT(5))
-#define UARTDM_RXFS 0x50 /* RX channel status register */
-#define UARTDM_RXFS_BUF_SHIFT 0x7 /* Number of bytes in the packing buffer
*/
-#define UARTDM_RXFS_BUF_MASK 0x7
#define UARTDM_MR1 0x00
#define UARTDM_MR1_RX_RDY_CTL BIT(7)
#define UARTDM_MR2 0x04
@@ -45,118 +41,56 @@
#define UARTDM_CSR 0xA0
#define UARTDM_SR 0xA4 /* Status register */
-#define UARTDM_SR_RX_READY (1 << 0) /* Word is the receiver FIFO */
+#define UARTDM_SR_RX_READY (1 << 0) /* Receiver FIFO has data */
+#define UARTDM_SR_TX_READY (1 << 2) /* Transmitter FIFO has space */
#define UARTDM_SR_TX_EMPTY (1 << 3) /* Transmitter underrun */
-#define UARTDM_SR_UART_OVERRUN (1 << 4) /* Receive overrun */
#define UARTDM_CR 0xA8 /* Command register */
#define UARTDM_CR_RX_ENABLE (1 << 0) /* Enable receiver */
#define UARTDM_CR_TX_ENABLE (1 << 2) /* Enable transmitter */
#define UARTDM_CR_CMD_RESET_RX (1 << 4) /* Reset receiver */
#define UARTDM_CR_CMD_RESET_TX (2 << 4) /* Reset transmitter */
-#define UARTDM_CR_CMD_RESET_ERR (3 << 4) /* Clear overrun error */
-#define UARTDM_CR_CMD_RESET_STALE_INT (8 << 4) /* Clears stale irq */
-#define UARTDM_CR_CMD_RESET_TX_READY (3 << 8) /* Clears TX Ready irq*/
-#define UARTDM_CR_CMD_FORCE_STALE (4 << 8) /* Causes stale event */
-#define UARTDM_CR_CMD_STALE_EVENT_DISABLE (6 << 8) /* Disable stale event */
-
-#define UARTDM_IMR 0xB0 /* Interrupt mask register */
-#define UARTDM_ISR 0xB4 /* Interrupt status register */
-#define UARTDM_ISR_TX_READY 0x80 /* TX FIFO empty */
#define UARTDM_TF 0x100 /* UART Transmit FIFO register */
#define UARTDM_RF 0x140 /* UART Receive FIFO register */
+#define UARTDM_RF_CHAR 0xff /* higher bits contain error information
*/
DECLARE_GLOBAL_DATA_PTR;
struct msm_serial_data {
phys_addr_t base;
- unsigned chars_cnt; /* number of buffered chars */
- uint32_t chars_buf; /* buffered chars */
uint32_t clk_rate; /* core clock rate */
};
-static int msm_serial_fetch(struct udevice *dev)
-{
- struct msm_serial_data *priv = dev_get_priv(dev);
- unsigned sr;
-
- if (priv->chars_cnt)
- return priv->chars_cnt;
-
- /* Clear error in case of buffer overrun */
- if (readl(priv->base + UARTDM_SR) & UARTDM_SR_UART_OVERRUN)
- writel(UARTDM_CR_CMD_RESET_ERR, priv->base + UARTDM_CR);
-
- /* We need to fetch new character */
- sr = readl(priv->base + UARTDM_SR);
-
- if (sr & UARTDM_SR_RX_READY) {
- /* There are at least 4 bytes in fifo */
- priv->chars_buf = readl(priv->base + UARTDM_RF);
- priv->chars_cnt = 4;
- } else {
- /* Check if there is anything in fifo */
- priv->chars_cnt = readl(priv->base + UARTDM_RXFS);
- /* Extract number of characters in UART packing buffer*/
- priv->chars_cnt = (priv->chars_cnt >>
- UARTDM_RXFS_BUF_SHIFT) &
- UARTDM_RXFS_BUF_MASK;
- if (!priv->chars_cnt)
- return 0;
-
- /* There is at least one charcter, move it to fifo */
- writel(UARTDM_CR_CMD_FORCE_STALE,
- priv->base + UARTDM_CR);
-
- priv->chars_buf = readl(priv->base + UARTDM_RF);
- writel(UARTDM_CR_CMD_RESET_STALE_INT,
- priv->base + UARTDM_CR);
- writel(0x7, priv->base + UARTDM_DMRX);
- }
-
- return priv->chars_cnt;
-}
-
static int msm_serial_getc(struct udevice *dev)
{
struct msm_serial_data *priv = dev_get_priv(dev);
- char c;
- if (!msm_serial_fetch(dev))
+ if (!(readl(priv->base + UARTDM_SR) & UARTDM_SR_RX_READY))
return -EAGAIN;
- c = priv->chars_buf & 0xFF;
- priv->chars_buf >>= 8;
- priv->chars_cnt--;
-
- return c;
+ return readl(priv->base + UARTDM_RF) & UARTDM_RF_CHAR;
}
static int msm_serial_putc(struct udevice *dev, const char ch)
{
struct msm_serial_data *priv = dev_get_priv(dev);
- if (!(readl(priv->base + UARTDM_SR) & UARTDM_SR_TX_EMPTY) &&
- !(readl(priv->base + UARTDM_ISR) & UARTDM_ISR_TX_READY))
+ if (!(readl(priv->base + UARTDM_SR) & UARTDM_SR_TX_READY))
return -EAGAIN;
- writel(UARTDM_CR_CMD_RESET_TX_READY, priv->base + UARTDM_CR);
-
- writel(1, priv->base + UARTDM_NCF_TX);
writel(ch, priv->base + UARTDM_TF);
-
return 0;
}
static int msm_serial_pending(struct udevice *dev, bool input)
{
- if (input) {
- if (msm_serial_fetch(dev))
- return 1;
- }
+ struct msm_serial_data *priv = dev_get_priv(dev);
- return 0;
+ if (input)
+ return !!(readl(priv->base + UARTDM_SR) & UARTDM_SR_RX_READY);
+ else
+ return !(readl(priv->base + UARTDM_SR) & UARTDM_SR_TX_EMPTY);
}
static const struct dm_serial_ops msm_serial_ops = {
@@ -222,8 +156,8 @@ static void uart_dm_init(struct msm_serial_data *priv)
writel(UARTDM_MR1_RX_RDY_CTL, priv->base + UARTDM_MR1);
writel(UARTDM_MR2_8_N_1_MODE, priv->base + UARTDM_MR2);
- /* Make sure BAM/single character mode is disabled */
- writel(0x0, priv->base + UARTDM_DMEN);
+ /* Enable single character mode */
+ writel(UARTDM_DMEN_TXRX_SC_ENABLE, priv->base + UARTDM_DMEN);
writel(UARTDM_CR_CMD_RESET_RX, priv->base + UARTDM_CR);
writel(UARTDM_CR_CMD_RESET_TX, priv->base + UARTDM_CR);
@@ -320,13 +254,9 @@ static inline void _debug_uart_putc(int ch)
{
struct msm_serial_data *priv = &init_serial_data;
- while (!(readl(priv->base + UARTDM_SR) & UARTDM_SR_TX_EMPTY) &&
- !(readl(priv->base + UARTDM_ISR) & UARTDM_ISR_TX_READY))
+ while (!(readl(priv->base + UARTDM_SR) & UARTDM_SR_TX_READY))
;
- writel(UARTDM_CR_CMD_RESET_TX_READY, priv->base + UARTDM_CR);
-
- writel(1, priv->base + UARTDM_NCF_TX);
writel(ch, priv->base + UARTDM_TF);
}
--
2.49.0
