linux-kernel - Re: [RFC] tty/serial/kgdboc: Add and wire up clear

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120912070154.GA8932@lizard>
Date:	Wed, 12 Sep 2012 00:01:54 -0700
From:	Anton Vorontsov <anton.vorontsov@...aro.org>
To:	Colin Cross <ccross@...roid.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Russell King <linux@....linux.org.uk>,
	Jason Wessel <jason.wessel@...driver.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Alan Cox <alan@...ux.intel.com>,
	Arve Hjønnevåg <arve@...roid.com>,
	Brian Swetland <swetland@...gle.com>,
	John Stultz <john.stultz@...aro.org>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linaro-kernel@...ts.linaro.org, patches@...aro.org,
	kernel-team@...roid.com, kgdb-bugreport@...ts.sourceforge.net,
	linux-serial@...r.kernel.org
Subject: Re: [RFC] tty/serial/kgdboc: Add and wire up clear_irqs callback

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/