UART receive error when data stream contains long breaks

[ch-f]

Describe the bug

UART (/dev/ttyAMBA0) erroneously receives only around 32 bytes of mostly incorrect data during transmissions containing long break signals.

Disabling DMA_ENGINE for amba-pl011 resolves the issue, ensuring accurate reception of all data (77 bytes).

Steps to reproduce the behaviour

Tx from host sending to Raspberry Pi 5:

• 7 times this pattern: long-break, 0x55, 0xe2, 0xf9, 0xf9, 0xf9, 0xf9, 0xf9, 0xf9, 0xf9, 0xf9, 0x30

Rx flowing into Raspberry Pi 5:

root@rpi5:~# od -v -w1 -t x1 /dev/ttyAMA0
0x00
0x00
0x55
0x80
0x00
0xf9
0xf9
0x78
0xf0
0xf9
0xc1
0xf9
0xf9
0xf9
0xf8
0xa0
0x95
0xfe
0x3c
0xe0
0xf9
0xf1
0xe4
0xf9
0xf9
0xf9
0xf9
0x01
0x00
0x55
0xe2
0xf9
0xf9

C test program for host to generate the Tx data (including long breaks):

#include <stdio.h>
#include <fcntl.h>
#include <errno.h>
#include <termios.h>
#include <unistd.h>
#include <string.h>

int set_speed(int fd, int speed) {
    struct termios tty;
    memset(&tty, 0, sizeof tty);
    if (tcgetattr(fd, &tty) != 0) {
        printf("Error %d from tcgetattr: %s\n", errno, strerror(errno));
        return -1;
    }
    cfsetospeed(&tty, speed);
    cfsetispeed(&tty, speed);
    if (tcsetattr(fd, TCSANOW, &tty) != 0) {
        printf("Error %d from tcsetattr: %s\n", errno, strerror(errno));
        return -1;
    }
    return 0;
}

int set_interface_attribs(int fd) {
    struct termios tty;
    if (tcgetattr(fd, &tty) != 0) {
        printf("Error %d from tcgetattr: %s\n", errno, strerror(errno));
        return -1;
    }

    tty.c_cflag &= ~(PARENB | PARODD | CRTSCTS | CSIZE);
    tty.c_cflag |= (CS8 | CLOCAL | CREAD);
    tty.c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP | INLCR | IGNCR | ICRNL | IXON);
    tty.c_oflag &= ~OPOST;
    tty.c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN);

    if (tcsetattr(fd, TCSANOW, &tty) != 0) {
        printf("Error %d from tcsetattr: %s\n", errno, strerror(errno));
        return -1;
    }
    return 0;
}

void send_lin_frame(int fd) {
    unsigned char lin_frame[] = {0x55, 0xe2, 0xf9, 0xf9, 0xf9, 0xf9, 0xf9, 0xf9, 0xf9, 0xf9, 0x30};
    write(fd, lin_frame, sizeof(lin_frame));
}

int main() {
    char *portname = "/dev/ttyUSB0";
    int fd = open(portname, O_RDWR | O_NOCTTY | O_SYNC);
    if (fd < 0) {
        printf("Error %d opening %s: %s\n", errno, portname, strerror(errno));
        return -1;
    }
    set_interface_attribs(fd);

    for (int i = 0; i < 7; i++) {
        set_speed(fd, B2400);
        write(fd, "\0", 1); // Send long break signal
        set_speed(fd, B9600);
        send_lin_frame(fd); // Send LIN frame
    }

    close(fd);
    return 0;
}

Device (s)

Raspberry Pi 5

System

Tested on a Raspberry Pi 5 with kernel from current branch rpi-6.8.y (6.8.0-rc5 + Raspberry patches).

root@rpi5:~# cat /etc/rpi-issue
Raspberry Pi reference 2023-12-05
Generated using pi-gen, https://github.com/RPi-Distro/pi-gen, 70cd6f2a1e34d07f5cba7047aea5b92457372e05, stage4

root@rpi5:~# vcgencmd version
2024/01/05 15:57:40 
Copyright (c) 2012 Broadcom
version 30cc5f37 (release) (embedded)

root@rpi5:~# uname -a
Linux rpi5 6.8.0-rc5 #1 SMP PREEMPT Fri Feb 23 23:38:34 CET 2024 aarch64 GNU/Linux

Logs

No response

Additional context

No response

[pelwell]

It seems to me that the RX DMA code in the PL011 driver is incompatible with break handling. The break indication is carried with the error flags in the top 4 bits of a 12-bit wide FIFO. The DMA RX code treats this as an 8-bit register, ignoring the flags, and hence ignoring/mishandling breaks. Disabling DMA (or at least in the RX path), something that can be achieved in DT - perhaps as a parameter - seems like the only sure way round this. Even if the DMA had a width of 16 or 32 bits, which would need post-processing to look for errors and extract the data, it's not clear if it would then work correctly.

[ch-f]

            if (ch & UART011_DR_BE)
                flag = TTY_BREAK;
            else if (ch & UART011_DR_PE)
                flag = TTY_PARITY;
            else if (ch & UART011_DR_FE)
                flag = TTY_FRAME;
        <snip>
            uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);

@pelwell Are you referring to this handling here?

Doesn't 'ch' get intentionally truncated (so that the data byte becomes 0x00) while BREAK information is carried within the 8-bit 'flag'?

It would be nice to identify the root cause before considering introduction of a dts flag to disable RX DMA. Do you think this issue should be escalated mainline?

[pelwell]

Yes, that's the non-DMA code that reads the full 12-bit values and looks at the flags. The DMA path just reads the lower 8 bits.