All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anton Vorontsov <anton.vorontsov@linaro.org>
To: Alan Cox <alan@linux.intel.com>
Cc: "Colin Cross" <ccross@android.com>,
	"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>,
	"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: Wed, 12 Sep 2012 03:32:05 -0700	[thread overview]
Message-ID: <20120912103202.GA26690@lizard> (raw)
In-Reply-To: <20120912104420.06a88957@bob.linux.org.uk>

On Wed, Sep 12, 2012 at 10:44:20AM +0100, Alan Cox wrote:
> > Of course, if Alan is OK with this, I'm more than OK too. :-)
> 
> It may well be better.
> 
> > (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 a lot of devices and platforms you'd probably mask them instead ?

If there is no way to clear them, yes, we obviously would want to
mask them before using the port for NMI debugger. Then we'd need
three callbacks:

- mask_all_irqs_but_rx() -- used before we want to start using the port
  for the NMI debugger;
- clear_rx_irq() -- (optional) clears rx IRQ for controllers that need
  it;
- restore_irqs() -- unmasks interrupts that were previously masked.

If we ever encounter a case when just clearing interrupts doesn't work,
we can surely implement the above scheme... It's just so far I don't
see any need to over-design this, but again, it's your call, I told my
opinion on this, but I'll do whatever you guys like more. :-)

> > > 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.
> 
> Recursively or not... again you get platform specific magic in places
> we don't want.

I really really don't see how this is platform-specific. All we ask the
serial driver is to quiesce its interrupt. Whether we can handle
NMIs/IRQs recursively or not is not serial driver's worry, since its
IRQ handler is not going to fire anyway. The polling routines already
gave us the power to steal/inject the data, so now we're stealing the
interrupt too.

How we use the callback is indeed platform-specific, but so is the
whole KGDB, and that knowledge is hidden there.

For serial driver it's all pretty much clear: lower the interrupt, but
don't turn off rx detection.

Thanks!

Anton.

WARNING: multiple messages have this Message-ID (diff)
From: anton.vorontsov@linaro.org (Anton Vorontsov)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC] tty/serial/kgdboc: Add and wire up clear_irqs callback
Date: Wed, 12 Sep 2012 03:32:05 -0700	[thread overview]
Message-ID: <20120912103202.GA26690@lizard> (raw)
In-Reply-To: <20120912104420.06a88957@bob.linux.org.uk>

On Wed, Sep 12, 2012 at 10:44:20AM +0100, Alan Cox wrote:
> > Of course, if Alan is OK with this, I'm more than OK too. :-)
> 
> It may well be better.
> 
> > (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 a lot of devices and platforms you'd probably mask them instead ?

If there is no way to clear them, yes, we obviously would want to
mask them before using the port for NMI debugger. Then we'd need
three callbacks:

- mask_all_irqs_but_rx() -- used before we want to start using the port
  for the NMI debugger;
- clear_rx_irq() -- (optional) clears rx IRQ for controllers that need
  it;
- restore_irqs() -- unmasks interrupts that were previously masked.

If we ever encounter a case when just clearing interrupts doesn't work,
we can surely implement the above scheme... It's just so far I don't
see any need to over-design this, but again, it's your call, I told my
opinion on this, but I'll do whatever you guys like more. :-)

> > > 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.
> 
> Recursively or not... again you get platform specific magic in places
> we don't want.

I really really don't see how this is platform-specific. All we ask the
serial driver is to quiesce its interrupt. Whether we can handle
NMIs/IRQs recursively or not is not serial driver's worry, since its
IRQ handler is not going to fire anyway. The polling routines already
gave us the power to steal/inject the data, so now we're stealing the
interrupt too.

How we use the callback is indeed platform-specific, but so is the
whole KGDB, and that knowledge is hidden there.

For serial driver it's all pretty much clear: lower the interrupt, but
don't turn off rx detection.

Thanks!

Anton.

  reply	other threads:[~2012-09-12 10:32 UTC|newest]

Thread overview: 49+ 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:30 ` Anton Vorontsov
2012-09-11  9:34 ` [PATCH 01/12] kernel/debug: Mask KGDB NMI upon entry Anton Vorontsov
2012-09-11  9:34   ` Anton Vorontsov
2012-09-11  9:34 ` [PATCH 02/12] kdb: Implement disable_nmi command Anton Vorontsov
2012-09-11  9:34   ` Anton Vorontsov
2012-09-11  9:34   ` 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   ` Anton Vorontsov
2012-09-11  9:34   ` Anton Vorontsov
2012-09-11  9:34 ` [PATCH 04/12] tty/serial/core: Introduce poll_init callback Anton Vorontsov
2012-09-11  9:34   ` Anton Vorontsov
2012-09-11  9:34 ` [PATCH 05/12] tty/serial/amba-pl011: Implement " Anton Vorontsov
2012-09-11  9:34   ` Anton Vorontsov
2012-09-11  9:34   ` 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  9:35   ` Anton Vorontsov
2012-09-11 14:15   ` Alan Cox
2012-09-11 14:15     ` Alan Cox
2012-09-12  3:32     ` [RFC] " Anton Vorontsov
2012-09-12  3:32       ` Anton Vorontsov
2012-09-12  3:42       ` Colin Cross
2012-09-12  3:42         ` Colin Cross
2012-09-12  4:06         ` Anton Vorontsov
2012-09-12  4:06           ` Anton Vorontsov
2012-09-12  4:40           ` Colin Cross
2012-09-12  4:40             ` Colin Cross
2012-09-12  7:01             ` Anton Vorontsov
2012-09-12  7:01               ` Anton Vorontsov
2012-09-12  9:44           ` Alan Cox
2012-09-12  9:44             ` Alan Cox
2012-09-12 10:32             ` Anton Vorontsov [this message]
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   ` Anton Vorontsov
2012-09-11  9:35 ` [PATCH 08/12] tty/serial: Add kgdb_nmi driver Anton Vorontsov
2012-09-11  9:35   ` Anton Vorontsov
2012-09-11 14:14   ` Alan Cox
2012-09-11 14:14     ` Alan Cox
2012-09-12  0:41     ` Anton Vorontsov
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   ` 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   ` 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   ` Anton Vorontsov
2012-09-11  9:35 ` [PATCH 12/12] ARM: versatile: Make able to use UART ports for KGDB FIQ debugger Anton Vorontsov
2012-09-11  9:35   ` 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=20120912103202.GA26690@lizard \
    --to=anton.vorontsov@linaro.org \
    --cc=akpm@linux-foundation.org \
    --cc=alan@linux.intel.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=arve@android.com \
    --cc=ccross@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.