Thread (7 messages) 7 messages, 3 authors, 2020-01-21

Re: [PATCH] tty: serial: amba-pl011: added RS485 support

From: Lukas Wunner <lukas@wunner.de>
Date: 2020-01-21 09:00:04
Also in: lkml

On Fri, Jan 17, 2020 at 01:58:57AM +0100, Ivan Šištík - 3K Solutions, s. r. o. wrote:
On 16. 1. 2020 at 14:29 Lukas Wunner wrote:
quoted
So I've implemented rs485 support for amba-pl011.c two years ago
but the patches need a little more polishing before they can be
upstreamed and I haven't gotten around to that yet.  I apologize
that it meant you had to reinvent the wheel.
You can find my implementation on this branch:
https://github.com/RevolutionPi/linux/commits/revpi-4.19

Specifically this commit:
https://github.com/RevolutionPi/linux/commit/0099313962a5
The wheel with octagonal shape is still not perfect. I made it more
smoother. Your implementation in recommended commit use an active
waiting (pl011_rs485_tx_start, pl011_rs485_tx_stop) and that could
cause lots of problems in upper layers of tty driver or application.
I think you forgot to implement possibility to start TX during
"delay after send", too.
Are these delays ever set to a value > 0 in practice?  struct serial_rs485
supports millisecond delays, but all RS485 transceivers I know of only
require delays in the microsecond or nanosecond range.  It was likely
a mistake that the delays were originally declared as millisecond values.
However we can't easily change that now because it's ABI and thus set in
stone.

E.g. the MAXM22510 has a "Driver Enable to Output" delay of 2.540 usec.
In practice no delay is necessary at all because the MMIO operations
performed by the driver take longer:

https://datasheets.maximintegrated.com/en/ds/MAXM22510-MAXM22511.pdf

quoted
I took a completely different approach:  I converted amba-pl011.c
to threaded interrupt handling using two kthreads, one for sending,
one for receiving.  This allows simultaneous writing to and reading
from the FIFO.  The driver keeps track of the FIFO fill level,
which allows writing to the FIFO blindly.  The hardirq handler
updates the fill level counter and wakes either of the IRQ threads.
I do not see any used thread in link:
https://github.com/RevolutionPi/linux/commit/0099313962a5
I am not kernel thread expert but I think that thread is not as
lightweight as hrtimer. According to my knowledge the hrtimer use some
kind of interrupt. Compare to this the kthread is created as thread
with all its scheduling structures. Did you implemented proper thread
shutdown? Has the thread right priority? There are many questions
like this...
You're not seeing the conversion to threaded IRQ handling because it's
in separate commits on the above-linked branch, e.g.

serial: pl011: Use threaded interrupt for RX
https://github.com/RevolutionPi/linux/commit/4f3a6e9ea335

serial: pl011: Use threaded interrupt for TX
https://github.com/RevolutionPi/linux/commit/fae65b5a2c5b

I implemented threaded IRQ handling to maximize TX throughput
and minimize RX FIFO overruns at high baudrates.  Additionally,
threaded IRQ handling integrates more nicely with the realtime
patch set.  So the ability to simply use msleep() for rs485 delays
is merely a by-product.

The IRQ threads run at RT priority -50 with SCHED_FIFO policy just
as any other IRQ thread and user space may adjust that with chrt(1)
if desired.

Thanks,

Lukas
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help