On Tuesday 11 September 2018 08:35 AM, Darwin Dingel wrote:
> When a serial port gets faulty or gets flooded with inputs, its interrupt
> handler starts to work double time to get the characters to the workqueue
> for the tty layer to handle them. When this busy time on the serial/tty
> subsystem happens during boot, where it is also busy on the userspace
> trying to initialise, some processes can continuously get preempted
> and will be on hold until the interrupts subside.
>
> The fix is to backoff on processing received characters for a specified
> amount of time when an input overrun is seen (received a new character
> before the previous one is processed).
You must really be using HW flow control to avoid HW FIFO overrruns.
> This only stops receive and will
> continue to transmit characters to serial port. After the backoff period
> is done, it receive will be re-enabled. This is optional and will only
> be enabled by setting 'overrun-throttle-ms' in the dts.
>
Is this binding documented? If not, this new binding should be
documented under Documentation/devicetree/bindings/serial/serial.txt and
with DT maintainer's ACK
Also, please CC maintainers listed by running
./scripts/get_maintainer.pl on the patches you submit. Else, you may not
get their attention quickly.
Regards
Vignesh
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Signed-off-by: Darwin Dingel <darwin.dingel@xxxxxxxxxxxxxxxxxxx>
> ---
> drivers/tty/serial/8250/8250_core.c | 25 +++++++++++++++++++++++++
> drivers/tty/serial/8250/8250_fsl.c | 23 ++++++++++++++++++++++-
> drivers/tty/serial/8250/8250_of.c | 5 +++++
> include/linux/serial_8250.h | 4 ++++
> 4 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 8fe3d0ed229e..0e65d4261f94 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -946,6 +946,21 @@ static struct uart_8250_port *serial8250_find_match_or_unused(struct uart_port *
> return NULL;
> }
>
> +static void serial_8250_overrun_backoff_work(struct work_struct *work)
> +{
> + struct uart_8250_port *up =
> + container_of(to_delayed_work(work), struct uart_8250_port,
> + overrun_backoff);
> + struct uart_port *port = &up->port;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&port->lock, flags);
> + up->ier |= UART_IER_RLSI | UART_IER_RDI;
> + up->port.read_status_mask |= UART_LSR_DR;
> + serial_out(up, UART_IER, up->ier);
> + spin_unlock_irqrestore(&port->lock, flags);
> +}
> +
> /**
> * serial8250_register_8250_port - register a serial port
> * @up: serial port template
> @@ -1060,6 +1075,16 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
> ret = 0;
> }
> }
> +
> + /* Initialise interrupt backoff work if required */
> + if (up->overrun_backoff_time_ms > 0) {
> + uart->overrun_backoff_time_ms = up->overrun_backoff_time_ms;
> + INIT_DELAYED_WORK(&uart->overrun_backoff,
> + serial_8250_overrun_backoff_work);
> + } else {
> + uart->overrun_backoff_time_ms = 0;
> + }
> +
> mutex_unlock(&serial_mutex);
>
> return ret;
> diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c
> index 6640a4c7ddd1..bb9571eed275 100644
> --- a/drivers/tty/serial/8250/8250_fsl.c
> +++ b/drivers/tty/serial/8250/8250_fsl.c
> @@ -45,8 +45,29 @@ int fsl8250_handle_irq(struct uart_port *port)
>
> lsr = orig_lsr = up->port.serial_in(&up->port, UART_LSR);
>
> - if (lsr & (UART_LSR_DR | UART_LSR_BI))
> + /* Process incoming characters first */
> + if ((lsr & (UART_LSR_DR | UART_LSR_BI)) &&
> + (up->ier & (UART_IER_RLSI | UART_IER_RDI))) {
> lsr = serial8250_rx_chars(up, lsr);
> + }
> +
> + /* Stop processing interrupts on input overrun */
> + if ((orig_lsr & UART_LSR_OE) && (up->overrun_backoff_time_ms > 0)) {
> + unsigned long delay;
> +
> + up->ier = port->serial_in(port, UART_IER);
> + if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) {
> + port->ops->stop_rx(port);
> + } else {
> + /* Keep restarting the timer until
> + * the input overrun subsides.
> + */
> + cancel_delayed_work(&up->overrun_backoff);
> + }
> +
> + delay = msecs_to_jiffies(up->overrun_backoff_time_ms);
> + schedule_delayed_work(&up->overrun_backoff, delay);
> + }
>
> serial8250_modem_status(up);
>
> diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
> index af8beefe9b5c..2a3244067ed6 100644
> --- a/drivers/tty/serial/8250/8250_of.c
> +++ b/drivers/tty/serial/8250/8250_of.c
> @@ -236,6 +236,11 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
> if (of_property_read_bool(ofdev->dev.of_node, "auto-flow-control"))
> port8250.capabilities |= UART_CAP_AFE;
>
> + if (of_property_read_u32(ofdev->dev.of_node,
> + "overrun-throttle-ms",
> + &port8250.overrun_backoff_time_ms) != 0)
> + port8250.overrun_backoff_time_ms = 0;
> +
> ret = serial8250_register_8250_port(&port8250);
> if (ret < 0)
> goto err_dispose;
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index 18e21427bce4..5a655ba8d273 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -134,6 +134,10 @@ struct uart_8250_port {
> void (*dl_write)(struct uart_8250_port *, int);
>
> struct uart_8250_em485 *em485;
> +
> + /* Serial port overrun backoff */
> + struct delayed_work overrun_backoff;
> + u32 overrun_backoff_time_ms;
> };
>
> static inline struct uart_8250_port *up_to_u8250p(struct uart_port *up)
>
