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 00:01:54 -0700 [thread overview]
Message-ID: <20120912070154.GA8932@lizard> (raw)
In-Reply-To: <CAMbhsRT+JR9-qzADiPXo9e59oHFCY4g6HNObKNMeciC5Xj6m6g@mail.gmail.com>
On Tue, Sep 11, 2012 at 09:40:35PM -0700, Colin Cross wrote:
[..]
> >> 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).
Yes, but if you'd like to merge your code, some might ask you: why?
You'd answer that you need to clear the interrupts, otherwise you'll
keep reentering NMI. The next that you might get is this: "this does
not belong to the getc callback, it's better to factor it out". :-) And
here comes clear_irq() (or alike, see below).
> >> 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.
Yup, and that's exactly what we do: http://lkml.org/lkml/2012/9/11/119
Your words made me think that clear_irq() might be indeed a somewhat
inappropriate name. We have to be even stricter on its definition and
behaviour.
So, returning to your question "What does clear_irqs() mean", I'd
answer that: the function must do whatever needed to lower the IRQ
line, plus the function must leave the port in the state that it's
still able to throw RX interrupts after the call.
So, the 100% proper name for this function would be this:
quiesce_irqs_but_rx()
It's a bit long, but does exactly what the name states.
Thanks!
Anton.
next prev parent reply other threads:[~2012-09-12 7:01 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
2012-09-12 7:01 ` Anton Vorontsov [this message]
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=20120912070154.GA8932@lizard \
--to=anton.vorontsov@linaro.org \
--cc=linux-arm-kernel@lists.infradead.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).