From: Jon Hunter <jonathanh@nvidia.com>
To: Wander Costa <wcosta@redhat.com>
Cc: Jiri Slaby <jirislaby@kernel.org>,
Wander Lairson Costa <wander@redhat.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Maciej W. Rozycki" <macro@orcam.me.uk>,
Johan Hovold <johan@kernel.org>, Andrew Jeffery <andrew@aj.id.au>,
"open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>,
open list <linux-kernel@vger.kernel.org>,
"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Subject: Re: [PATCH] tty: serial: Use fifo in 8250 console driver
Date: Tue, 25 Jan 2022 12:40:27 +0000 [thread overview]
Message-ID: <fa42a60c-954a-acc0-3962-f00427153f78@nvidia.com> (raw)
In-Reply-To: <CAAq0SU=9R3Y_SAdM+HaqavzWBRd1Li-b5bnZZLd5Opfgd0vnkQ@mail.gmail.com>
On 25/01/2022 10:29, Wander Costa wrote:
> On Tue, Jan 25, 2022 at 7:06 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>>
>> On 25/01/2022 09:36, Jiri Slaby wrote:
>>
>> ...
>>
>>>> The test is bogus:
>>>> use_fifo = (up->capabilities & UART_CAP_FIFO) &&
>>>> port->fifosize > 1 &&
>>>> (serial_port_in(port, UART_FCR) & UART_FCR_ENABLE_FIFO)
>>>>
>>>> FCR is write only. Reading it, one gets IIR contents.
>>>
>>> In particular, the test is checking whether there is no interrupt
>>> pending (UART_FCR_ENABLE_FIFO == UART_IIR_NO_INT). So it oscillates
>>> between use_fifo and not, depending on the interrupt state of the chip.
>>>
>>> Could you change it into something like this:
>>> --- a/drivers/tty/serial/8250/8250_port.c
>>> +++ b/drivers/tty/serial/8250/8250_port.c
>>> @@ -3396,7 +3396,7 @@ void serial8250_console_write(struct
>>> uart_8250_port *up, const char *s,
>>>
>>> use_fifo = (up->capabilities & UART_CAP_FIFO) &&
>>> port->fifosize > 1 &&
>>> - (serial_port_in(port, UART_FCR) & UART_FCR_ENABLE_FIFO) &&
>>> + (up->fcr & UART_FCR_ENABLE_FIFO) &&
>>> /*
>>> * After we put a data in the fifo, the controller will
>>> send
>>> * it regardless of the CTS state. Therefore, only use
>>> fifo
>>>
>>>
>>> And see whether it fixes the issue. Anyway, of what port type is the
>>> serial port (what says dmesg/setserial about that)?
>>
>>
>> Thanks. Unfortunately, this did not fix it. The port type is PORT_TEGRA ...
>>
>> 70006000.serial: ttyS0 at MMIO 0x70006000 (irq = 72, base_baud = 25500000) is a Tegra
>
> I see PORT_TEGRA has different values for fifosize and tx_loadsz.
> Maybe we should use tx_loadsz.
> Could you please give a try to this patch:
>
> diff --git a/drivers/tty/serial/8250/8250_port.c
> b/drivers/tty/serial/8250/8250_port.c
> index 2abb3de11a48..d3a93e5d55f7 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -3343,7 +3343,7 @@ static void serial8250_console_fifo_write(struct
> uart_8250_port *up,
> {
> int i;
> const char *end = s + count;
> - unsigned int fifosize = up->port.fifosize;
> + unsigned int fifosize = up->tx_loadsz;
> bool cr_sent = false;
>
> while (s != end) {
> @@ -3409,8 +3409,8 @@ void serial8250_console_write(struct
> uart_8250_port *up, const char *s,
> }
>
> use_fifo = (up->capabilities & UART_CAP_FIFO) &&
> - port->fifosize > 1 &&
> - (serial_port_in(port, UART_FCR) & UART_FCR_ENABLE_FIFO) &&
> + up->tx_loadsz > 1 &&
> + (up->fcr & UART_FCR_ENABLE_FIFO) &&
> /*
> * After we put a data in the fifo, the controller will send
> * it regardless of the CTS state. Therefore, only use fifo
>
Thanks. Yes that does fix it.
Andy, does this work for X86?
Jon
--
nvpublic
next prev parent reply other threads:[~2022-01-25 12:43 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-29 20:14 [PATCH] tty: serial: Use fifo in 8250 console driver wander
[not found] ` <CAHp75VeZBp4gKvGBDzaD=EpGRDZ1-wTvD8K9Ui6Q59kDjmkXmQ@mail.gmail.com>
2021-11-01 15:22 ` Wander Costa
2021-11-01 15:32 ` Andy Shevchenko
2021-11-10 12:10 ` Wander Costa
2021-11-12 11:58 ` David Laight
2022-01-25 8:39 ` Jon Hunter
2022-01-25 8:50 ` Greg Kroah-Hartman
2022-01-25 9:03 ` Jon Hunter
2022-01-25 9:08 ` Jiri Slaby
2022-01-25 9:36 ` Jiri Slaby
2022-01-25 10:06 ` Jon Hunter
2022-01-25 10:29 ` Wander Costa
2022-01-25 12:40 ` Jon Hunter [this message]
2022-01-25 16:53 ` Andy Shevchenko
2022-01-25 16:54 ` Andy Shevchenko
2022-01-25 18:40 ` Wander Costa
2022-01-26 8:52 ` Greg Kroah-Hartman
2022-01-26 12:09 ` Andy Shevchenko
2022-01-26 13:23 ` Wander Costa
2022-01-26 13:34 ` Andy Shevchenko
2022-01-27 18:10 ` Theodore Y. Ts'o
2022-01-25 10:18 ` Wander Costa
2022-01-25 10:38 ` Jiri Slaby
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=fa42a60c-954a-acc0-3962-f00427153f78@nvidia.com \
--to=jonathanh@nvidia.com \
--cc=andrew@aj.id.au \
--cc=andriy.shevchenko@linux.intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@kernel.org \
--cc=johan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=macro@orcam.me.uk \
--cc=wander@redhat.com \
--cc=wcosta@redhat.com \
/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).