public inbox for linux-serial@vger.kernel.org
 help / color / mirror / Atom feed
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: Min Zhang <mzhang@mvista.com>
Cc: gregkh@linuxfoundation.org, linux-serial@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] serial: 8250 check iir rdi in interrupt
Date: Tue, 30 Oct 2012 14:24:57 +0000	[thread overview]
Message-ID: <20121030142457.00840d63@pyramind.ukuu.org.uk> (raw)
In-Reply-To: <CAJ0e-Fqzq6s6H=9CMpqJKug2=oR89_qw_52Yz-ihJd-8Bc-_+Q@mail.gmail.com>

On Fri, 26 Oct 2012 14:57:31 -0700
Min Zhang <mzhang@mvista.com> wrote:

> On Fri, Oct 26, 2012 at 7:19 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> > So we only need to check this in serial8250_handle_irq when IIR indicates
> > a data timeout interrupt ?
> >
> > Can we do
> >
> >         if ((iir & 0x0F) == 0x0C) {
> >                 /* Expensive RDI check */
> >         }
> >
> >
> >
> > Alan
> 
> Checking data timeout interrupt is only for "too much work for irq"
> problem. There is another console freeze problem which is caused by
> reading receive FIFO when there is no RDI interrupt, such as during
> THRI transmit interrupt, so no timeout interrupt. This time one has to
> check two both IIR and LSR. After all these checking, it becomes the
> original patch anyway.
> 
> I am posting another simpler revision to exclude timer handler from
> this workaround, and make this workaround default off and inline.

We have two problems one of which is understood. Your patch continues to
do an extra read in an absolutely critical hot path and one that on many
systems will cost upwards of 8uS.

Can we therefore work through this neatly one problem at a time ?

In addition it seems your patch causes data loss if your check triggers
and a character appears during the check, at which point I don't see what
stops your RX read from dropping a byte of real user data ?

Alan

  reply	other threads:[~2012-10-30 14:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-23  0:19 [PATCH] serial: 8250 check iir rdi in interrupt Min Zhang
2012-10-23 10:01 ` Alan Cox
2012-10-23 19:43   ` Min Zhang
2012-10-26 14:19     ` Alan Cox
2012-10-26 21:57       ` Min Zhang
2012-10-30 14:24         ` Alan Cox [this message]
2012-10-26 22:16       ` [PATCHv2] " Min Zhang

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=20121030142457.00840d63@pyramind.ukuu.org.uk \
    --to=alan@lxorguk.ukuu.org.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=mzhang@mvista.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