linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Colin Cross <ccross@android.com>
To: Anton Vorontsov <anton.vorontsov@linaro.org>
Cc: "Thomas Gleixner" <tglx@linutronix.de>,
	"Alan Cox" <alan@lxorguk.ukuu.org.uk>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Russell King" <linux@arm.linux.org.uk>,
	"Jason Wessel" <jason.wessel@windriver.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Alan Cox" <alan@linux.intel.com>,
	"Arve Hjønnevåg" <arve@android.com>,
	"Brian Swetland" <swetland@google.com>,
	"John Stultz" <john.stultz@linaro.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linaro-kernel@lists.linaro.org, patches@linaro.org,
	kernel-team@android.com, kgdb-bugreport@lists.sourceforge.net,
	linux-serial@vger.kernel.org
Subject: Re: [RFC] tty/serial/kgdboc: Add and wire up clear_irqs callback
Date: Tue, 11 Sep 2012 21:40:35 -0700	[thread overview]
Message-ID: <CAMbhsRT+JR9-qzADiPXo9e59oHFCY4g6HNObKNMeciC5Xj6m6g@mail.gmail.com> (raw)
In-Reply-To: <20120912040657.GA3880@lizard>

On Tue, Sep 11, 2012 at 9:06 PM, Anton Vorontsov
<anton.vorontsov@linaro.org> wrote:
> On Tue, Sep 11, 2012 at 08:42:46PM -0700, Colin Cross wrote:
> [...]
>> > The "problem" is in the last step. If we exit NMI without making UART
>> > know that we're done with the interrupt, we will reenter the NMI
>> > immediately, even without any new characters from the UART.
>>
>> The UART irq line should go low when you read the character out of the
>
> Probably some controllers may lower the line by themselves, but not
> all, and probably most of them need an explicit clear.

Anything 8250-based will clear the interrupt automatically, assuming
you read the status registers as well as the character register.

>> receive buffer, or the polling rx function should clear the interrupt
>> for you.
>
> Yes, that's an option. But that way we add a new semantic for the
> polling routines, and effecitvely we just merge the two callbacks.
>
> Of course, if Alan is OK with this, I'm more than OK too. :-)
>
> (But the polling routines would need to clear all interrupts, not
> just rx/tx. For example, if the controller indicated some error, and
> nobody clears it, then we'll start reentering infinitely.)

For exynos5, the only non-8250 based serial port I've come across, we
clear all interrupts in the rx poll function (see
https://android.googlesource.com/kernel/exynos/+/ef427aafffb7153dde59745e440fd7ec41ea969d/arch/arm/mach-exynos/exynos_fiq_debugger.c).

>> If you use a clear_irqs callback, you can drop characters if
>> one arrives between the last character buffer read and calling
>> clear_irqs.
>
> Only if we call clear_irqs() after reading the characters, but we do
> it before. So if new characters are available, we will reenter NMI,
> which is OK.
>
> But if used incorrectly, it truly can cause dropping (or staling) of
> characters, so I'd better add some comments about this.

What does clear_irqs() mean for a status or tx interrupt?  The tx
interrupt will generally re-assert as long as the tx fifo is empty,
which would require disabling it.  On 8250 ports, status interrupts
will re-assert until the corresponding status register is read.

  reply	other threads:[~2012-09-12  4:40 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-11  9:30 [PATCH v6 0/12] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
2012-09-11  9:34 ` [PATCH 01/12] kernel/debug: Mask KGDB NMI upon entry Anton Vorontsov
2012-09-11  9:34 ` [PATCH 02/12] kdb: Implement disable_nmi command Anton Vorontsov
2012-09-11  9:34 ` [PATCH 03/12] kdb: Turn KGDB_KDB=n stubs into static inlines Anton Vorontsov
2012-09-11  9:34 ` [PATCH 04/12] tty/serial/core: Introduce poll_init callback Anton Vorontsov
2012-09-11  9:34 ` [PATCH 05/12] tty/serial/amba-pl011: Implement " Anton Vorontsov
2012-09-11  9:35 ` [PATCH 06/12] tty/serial/kgdboc: Add and wire up clear_irqs callback Anton Vorontsov
2012-09-11 14:15   ` Alan Cox
2012-09-12  3:32     ` [RFC] " Anton Vorontsov
2012-09-12  3:42       ` Colin Cross
2012-09-12  4:06         ` Anton Vorontsov
2012-09-12  4:40           ` Colin Cross [this message]
2012-09-12  7:01             ` Anton Vorontsov
2012-09-12  9:44           ` Alan Cox
2012-09-12 10:32             ` Anton Vorontsov
2012-09-11  9:35 ` [PATCH 07/12] tty/serial/amba-pl011: Implement " Anton Vorontsov
2012-09-11  9:35 ` [PATCH 08/12] tty/serial: Add kgdb_nmi driver Anton Vorontsov
2012-09-11 14:14   ` Alan Cox
2012-09-12  0:41     ` Anton Vorontsov
2012-09-11  9:35 ` [PATCH 09/12] ARM: Move some macros from entry-armv to entry-header Anton Vorontsov
2012-09-11  9:35 ` [PATCH 10/12] ARM: Add KGDB/KDB FIQ debugger generic code Anton Vorontsov
2012-09-11  9:35 ` [PATCH 11/12] ARM: VIC: Add a couple of low-level FIQ management helpers Anton Vorontsov
2012-09-11  9:35 ` [PATCH 12/12] ARM: versatile: Make able to use UART ports for KGDB FIQ debugger Anton Vorontsov

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=CAMbhsRT+JR9-qzADiPXo9e59oHFCY4g6HNObKNMeciC5Xj6m6g@mail.gmail.com \
    --to=ccross@android.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@linux.intel.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=anton.vorontsov@linaro.org \
    --cc=arve@android.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jason.wessel@windriver.com \
    --cc=john.stultz@linaro.org \
    --cc=kernel-team@android.com \
    --cc=kgdb-bugreport@lists.sourceforge.net \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=patches@linaro.org \
    --cc=swetland@google.com \
    --cc=tglx@linutronix.de \
    /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).