UART with CTS handshake locks up in latest (4.1.16) and earlier kernels

[Vlad-Apostolov]

When using the UART to continuously transmit data while changing often the CTS input, the UART locks up and stops transmitting. I was able to easily reproduce the problem with the WS2812B LED interface chip here:
https://www.kickstarter.com/projects/1338646202/simple-ws2812b-led-interface-controller-chip
when using it in Example 5. I managed to reproduce the problem with kernels 3.6.11 and 4.1.16.
I investigated the issue and found out that the kernel requires cts_event_workaround variable in the vendor_arm structure in amba-pl011.c to be initialized to true.
This issue has been reported and addressed for Linaro platforms in the past:
https://patches.linaro.org/1837/
https://patches.linaro.org/7461/
I built the 4.1.16 kernel with this fix and couldn’t reproduce the problem.

[P33M]

Is there a DT binding property or similar that sets this flag?

[Vlad-Apostolov]

No, I couldn't find one unfortunately.

linux/Documentation/devicetree/bindings/serial$ cat pl011.txt

• ARM AMBA Primecell PL011 serial UART

Required properties:

• compatible: must be "arm,primecell", "arm,pl011"
• reg: exactly one register range with length 0x1000
• interrupts: exactly one interrupt specifier

Optional properties:

• pinctrl:
  When present, must have one state named "default",
  and may contain a second name named "sleep". The former
  state sets up pins for ordinary operation whereas
  the latter state will put the associated pins to sleep
  when the UART is unused
• clocks:
  When present, the first clock listed must correspond to
  the clock named UARTCLK on the IP block, i.e. the clock
  to the external serial line, whereas the second clock
  must correspond to the PCLK clocking the internal logic
  of the block. Just listing one clock (the first one) is
  deprecated.
• clocks-names:
  When present, the first clock listed must be named
  "uartclk" and the second clock listed must be named
  "apb_pclk"
• dmas:
  When present, may have one or two dma channels.
  The first one must be named "rx", the second one
  must be named "tx".
• auto-poll:
  Enables polling when using RX DMA.
• poll-rate-ms:
  Rate at which poll occurs when auto-poll is set,
  default 100ms.
• poll-timeout-ms:
  Poll timeout when auto-poll is set, default
  3000ms.

See also bindings/arm/primecell.txt

Example:

uart@80120000 {
compatible = "arm,pl011", "arm,primecell";
reg = <0x80120000 0x1000>;
interrupts = <0 11 IRQ_TYPE_LEVEL_HIGH>;
dmas = <&dma 13 0 0x2>, <&dma 13 0 0x0>;
dma-names = "rx", "tx";
clocks = <&foo_clk>, <&bar_clk>;
clock-names = "uartclk", "apb_pclk";
};

[popcornmix]

No DT property for this (I've checked 4.5 kernel too in case it has been added more recently).
Seems a bit surprising that the driver supports a number of vendor specific config structures, rather than using DT.

I guess first question is are we 100% sure that cts_event_workaround is good for all Pi users and won't cause any regressions.

We could initially include it on 4.4 kernel / BRANCH=next and see if anyone reports issues.

[P33M]

I seem to remember pppd being flakey with rts/cts enabled when doing back-to-back Pi communication via the UART. This would be a good use-case to test with.

[tuwuhs]

Do we have any updates on this?

I used another MCU as receiving side (STM32F1), implementing simple polled read. I enabled hardware RTS in the serial peripheral in the STM32, connected to Pi's CTS line (GPIO16). On Pi side I continuously send bursts of 524 bytes, with ~30 ms pause in between. I used Raspberry Pi 3 Model B on my test.

What I found:

1. If I set the MCU to discard every byte it receives: Pi's UART locked up quickly, most of the time under 20 bursts.
2. If the MCU is programmed to echo every byte: I did not find any lock-up.
3. Applying @Vlad-Apostolov's fix (setting cts_event_workaround to true in vendor_arm structure in drivers/tty/serial/amba-pl011.c): I could not reproduce the lock up, running > 6 hours with no problem.

Tried on kernel 4.9.30 and 4.9.51, no difference.

I am quite positive that this workaround is needed for the UART peripheral in the BCM chip (not sure if this is actually a general issue in all ARM PrimeCell PL011 IP).

[pelwell]

I'm prepared to add support for a cts_event_workaround DT property in a downstream patch to the driver, and to set it in all downstream Pi DTBs. If that solves the problem and doesn't cause any regressions then we can raise the matter with the upstream maintainers.

[pelwell]

@popcornmix - I have an extremely simple patch that implements the above scheme, and its currently providing console output on ttyAMA0 with no obvious issues. Are you willing to accept it as is for rpi-update kernels, or would you prefer a dtparam to enable it until it's had more testing?

[popcornmix]

@pelwell No objections if you think that's the best option.

[pelwell]

I'll give it some more testing with Bluetooth this afternoon, then push it if I find nothing untoward.

[tuwuhs]

Thanks @pelwell. Let me know if you need help testing.

[pelwell]

Sorry it's taken so long, but I've pushed the patch now, along with a one-liner that makes UART TX reliable under load.

[tuwuhs]

No problem. I have tested it, worked perfectly. Thanks again!

[pelwell]

The latest rpi-update release includes this patch.