From: Rogier Wolff <R.E.Wolff@BitWizard.nl>
To: Dave Martin <Dave.Martin@arm.com>
Cc: Phil Elwell <phil@raspberrypi.org>,
Russell King <linux@arm.linux.org.uk>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jslaby@suse.com>,
"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
"linux-rpi-kernel@lists.infradead.org"
<linux-rpi-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] tty: amba-pl011: Make TX optimisation conditional
Date: Fri, 12 Jul 2019 14:10:00 +0200 [thread overview]
Message-ID: <20190712121000.GK11350@BitWizard.nl> (raw)
In-Reply-To: <20190712112105.GH2790@e103592.cambridge.arm.com>
On Fri, Jul 12, 2019 at 12:21:05PM +0100, Dave Martin wrote:
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 89ade21..1902071 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -1307,6 +1307,13 @@ static bool pl011_tx_chars(struct uart_amba_port *uap, bool from_irq);
> /* Start TX with programmed I/O only (no DMA) */
> static void pl011_start_tx_pio(struct uart_amba_port *uap)
> {
> + /*
> + * Avoid FIFO overfills if the TX IRQ is active:
> + * pl011_int() will comsume chars waiting in the xmit queue anyway.
> + */
> + if (uap->im & UART011_TXIM)
> + return;
> +
I'm no expert on PL011, have no knowledge of the current bug, but have
programmed serial drivers in the past.
This looks "dangerous" to me.
The normal situation is that you push the first few characters into
the FIFO with PIO and then the interrupt will trigger once the FIFO
empties and then you can refil the FIFO until the buffer empties.
The danger in THIS fix is that you might have a race that causes those
first few PIO-ed characters not to be put in the hardware resulting in
the interrupt never triggering.... If you can software-trigger the
interrupt just before the "return" here that'd be a way to fix things.
I'm ok with a reaction like "I've thought about this, it's not a
problem, now shut up".
Roger.
--
** R.E.Wolff@BitWizard.nl ** https://www.BitWizard.nl/ ** +31-15-2049110 **
** Delftechpark 11 2628 XJ Delft, The Netherlands. KVK: 27239233 **
The plan was simple, like my brother-in-law Phil. But unlike
Phil, this plan just might work.
next prev parent reply other threads:[~2019-07-12 12:18 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-11 13:45 [PATCH] tty: amba-pl011: Make TX optimisation conditional Phil Elwell
2019-07-12 11:21 ` Dave Martin
2019-07-12 11:53 ` Phil Elwell
2019-07-12 12:10 ` Rogier Wolff [this message]
2019-07-12 12:20 ` Phil Elwell
2019-07-12 16:35 ` Dave Martin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190712121000.GK11350@BitWizard.nl \
--to=r.e.wolff@bitwizard.nl \
--cc=Dave.Martin@arm.com \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rpi-kernel@lists.infradead.org \
--cc=linux-serial@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=phil@raspberrypi.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).