AM3356: PRU SUART driver issue

Hello Dachuan,

Bad news: I am not sure what PRU Software UART driver you received, but I cannot support that driver. A PRU SUART driver was written a long time ago, but it has not been supported for several years.

Good news: TI just wrote a new PRU Software UART driver! It is available for AM335x on Linux Processors SDK 6.1 and later. Please reference the PRU-ICSS Soft UART documentation.

Regards,

Nick

Hi Nick,

The PRU SUART driver I've mentioned is the new PRU SUART driver.

I've got it from TI FAE before SDK 6.1 been released. I compared the copy that I have and pru_suart.c in SDK 6.1, they are the same.

Hi Dachuan,

You mentioned the driver version you used is 7-12-2018 which confuses me, because the PRU SUART driver didn't exist in 2018.

Anyway, I don't believe the PRU SUART driver has been tested with v4.14 kernel, can you please test the Processor SDK v6.1 to see if you see the same kernel crash?

Hi LiuBin,

I've ported test to SDK 6.1 and reproduced the problem.

Can I send you email directly?

This issue is related with RT-Preempt patch's threaded interrupt handler mechanism.

The crash bug report shows suart driver's irq handler is in IRQ context, even if it's a full rt-preempt system. In the isr, the call stack is:

pport_handle_irq()
pport_rx_chars()
tty_flip_buffer_push() ==> which calls rt_spin_lock_slowlock_locked() then calls schedule()

all these functions are running in IRQ context, not thread context, so the preempt_count's hardirq bit is set to 1, schedule() triggers BUG_ON() report.

 In my opinion, the key to the problem is the IRQ handler is not threaded in RT-Linux.  When I change the irq_flag to IRQF_TRIGGER_RISING, the handler is registed as a kernel thread, the problem solved.

The patch file is attached, which contains another fix to driver probe issue.

If you confirm the problem, please let me know asap.

Thanks.

patch_pru_suart.zip

Hi Dachuan,

Thanks for the patch, but I am not sure it is the proper fix for the issue. It seems it only covers the issue probably due to timing change.

How do you do exactly to trigger the issue? I'd like to reproduce it on my setup and debug it.

And thanks for the DEFER probe fix in the patch, I will ensure the fix get into the next SDK release.

For my curiosity, why to do remove the nr_pins checking as in the patch?

Some functions called by pport_handle_irq() may be not IRQ safe, and should be called in bottom half of the interrupt.

The issue may be triggered by opening and sending/receiving mass data through ALL pru suart ports AT THE SAME TIME.

Our board has 3 pru suart ports, 4 built-in uart ports and 4 usb2serial ports. I create a test to use these ports simultaneously, it's very easy to reproduce the issue.

The test case is very simple as below:

int Test(int slave_id, int port_num)
{
    printf("hello, world\n");

    /* 打开串口, 非阻塞 */
    int serialfd = HAL_SerialOpen(port_num, 0);
    if (serialfd < 0)
    {
        printf("Open Serial Error. Serial No. = %d\n", port_num);
    }
    else
    {
        printf("Open Serial OK. Serial No. = %d\n", port_num);
    }

    /* 配置串口 */
    uint32_t baudrate = 9600;
    int bytesize = 8;
    char parity = 'E';
    int stopbits = 1;
    if (HAL_SerialConfigSet(serialfd, baudrate, bytesize, stopbits, parity) < 0)
    {
        printf("Config Serial Error\n");
        HAL_SerialClose(serialfd);
        return -1;
    }

    ReqInfo regs[] =
    {
        {0, 94},
        {94, 3},
        {122, 68},
        {1300, 34},
        {1400, 120},
        {1520, 78},
        {3000, 12},
        {3200, 12},
        {500, 18},
        {620, 106},
        {740, 106},
        {860, 106},
        {1600, 120},
        {1720, 78},
        {3418, 6},
        {3618, 6},
        {98, 11},
        {518, 88},
    };

    uint8_t sendbuf[100];
    DataBuf recvbuf;

    /* 错误计数 */
    int errorCnt = 0;

    size_t i = 0;
    while(1)
    {
        if (i >= sizeof(regs) / sizeof(regs[0]))
            i = 0;

        /* 组装请求帧 */
        int sendsz = makeFrame(slave_id, regs[i].regaddr, regs[i].regNum, sendbuf);
        i++;

        /* 发送数据 */
        sendData(serialfd, sendbuf, sendsz);

        /* 接收数据 */
        if (!recvData(serialfd, &recvbuf, 100, 50))
        {
            errorCnt++;
            if (errorCnt > 3)
                return -1;
        }
        else
        {
            errorCnt = 0;
        }

        printf("recv len = %d\n", recvbuf.recvLen);

        OS_TimeMsDelay(20);
    }

    return 0;
}

#include <thread>
using std::thread;

int main()
{
    std::thread serialThreads1(Test, SLAVE_ID, 1);
    std::thread serialThreads2(Test, SLAVE_ID, 2);
    std::thread serialThreads3(Test, SLAVE_ID, 3);
    std::thread serialThreads4(Test, SLAVE_ID, 4);
    std::thread serialThreads5(Test, SLAVE_ID, 5);
    std::thread serialThreads6(Test, SLAVE_ID, 6);
    std::thread serialThreads7(Test, SLAVE_ID, 7);
    std::thread serialThreads8(Test, SLAVE_ID, 8);
    std::thread serialThreads9(Test, SLAVE_ID, 9);
    std::thread serialThreads10(Test, SLAVE_ID, 10);
    std::thread serialThreads11(Test, SLAVE_ID, 11);

    serialThreads1.join();
    serialThreads2.join();
    serialThreads3.join();
    serialThreads4.join();
    serialThreads5.join();
    serialThreads6.join();
    serialThreads7.join();
    serialThreads8.join();
    serialThreads9.join();
    serialThreads10.join();
    serialThreads11.join();
}

About the nr_pins patch:

We modified the asm code to add RS-485 auto control function, the nr_pins may not be 2 or 4.

It's not related with the issue, but strongly recommand adding rs-485 support on PRU SUART for industrial communication.

--- b\pru_suart.c	2020-01-16 09:11:34 +0800
+++ a\pru_suart.c	2020-01-16 08:47:44 +0800
@@ -176,8 +176,12 @@
 			up->icount.parity++;
 		if (ch & PPORT_RX_CHAR_FE)
 			up->icount.frame++;
-
-		uart_insert_char(up, 0, 0, ch, TTY_NORMAL);
+		
+		/*cet, zhangyaqun, 190912, drop ch when PE/FE error occurs */
+		if(!(ch & PPORT_RX_CHAR_PE) && !(ch & PPORT_RX_CHAR_FE))
+		{
+			uart_insert_char(up, 0, 0, ch, TTY_NORMAL);
+		}
 	}
 
 	up->icount.rx += total;
@@ -580,7 +586,7 @@
 		return ret;
 	}
 
-	pu->pruss = pruss_get(pu->pru);
+	pu->pruss = pruss_get(pu->pru, NULL);
 	if (IS_ERR(pu->pruss)) {
 		ret = PTR_ERR(pu->pruss);
 		dev_err(pu->dev, "failed to get pruss handle (%d)\n", ret);

These are another two changes we made.

Hi Dachuan,

Dachuan Liu said:

Our board has 3 pru suart ports, 4 built-in uart ports and 4 usb2serial ports. I create a test to use these ports simultaneously, it's very easy to reproduce the issue.

I haven't got a chance to test this scenario on my side yet, but do you see the issue when only use the PRU SUART ports? At least not run with the usb2serial ports. Adding USB stack in the test case makes the data flow much more complicated.