[patch] Add insmod option to force the use of the backup timer.

[patch] Add insmod option to force the use of the backup timer.

[Gerd Hoffmann]

[-- Attachment #1: serial-8250-backup-timerPatch --]
[-- Type: text/plain, Size: 1813 bytes --]

The test which automatically enables the backup timer on some HP
machines doesn't trigger on other hardware which needs the backup
timer too.

This patch add a way to enable it manually (8250.use_backup_timer=1)
to deal with these machines.

Signed-off-by: Gerd Hoffmann <kraxel@suse.de>
Cc: Alex Williamson <alex.williamson@hp.com>
Cc: Kevin Stansell <kstansel@us.ibm.com>
---
 drivers/serial/8250.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Index: vanilla-2.6.21-rc2/drivers/serial/8250.c
===================================================================
--- vanilla-2.6.21-rc2.orig/drivers/serial/8250.c
+++ vanilla-2.6.21-rc2/drivers/serial/8250.c
@@ -52,8 +52,8 @@
  *                is unsafe when used on edge-triggered interrupts.
  */
 static unsigned int share_irqs = SERIAL8250_SHARE_IRQS;
-
 static unsigned int nr_uarts = CONFIG_SERIAL_8250_RUNTIME_UARTS;
+static unsigned int use_backup_timer;
 
 /*
  * Debugging.
@@ -1729,7 +1729,7 @@ static int serial8250_startup(struct uar
 		 * If the interrupt is not reasserted, setup a timer to
 		 * kick the UART on a regular basis.
 		 */
-		if (iir & UART_IIR_NO_INT) {
+		if (iir & UART_IIR_NO_INT || use_backup_timer) {
 			pr_debug("ttyS%d - using backup timer\n", port->line);
 			up->timer.function = serial8250_backup_timeout;
 			up->timer.data = (unsigned long)up;
@@ -2805,6 +2805,9 @@ module_param(share_irqs, uint, 0644);
 MODULE_PARM_DESC(share_irqs, "Share IRQs with other non-8250/16x50 devices"
 	" (unsafe)");
 
+module_param(use_backup_timer, uint, 0644);
+MODULE_PARM_DESC(use_backup_timer, "use backup timer");
+
 module_param(nr_uarts, uint, 0644);
 MODULE_PARM_DESC(nr_uarts, "Maximum number of UARTs supported. (1-" __MODULE_STRING(CONFIG_SERIAL_8250_NR_UARTS) ")");
 

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: [patch] Add insmod option to force the use of the backup timer.

[Dave Jones]

On Wed, Feb 28, 2007 at 11:23:46AM +0100, Gerd Hoffmann wrote:
 > The test which automatically enables the backup timer on some HP
 > machines doesn't trigger on other hardware which needs the backup
 > timer too.

Did you figure out *why* that test doesn't trigger?
Making that work seems a better solution to me than adding magic
options that users won't know they have to use.

		Dave

-- 
http://www.codemonkey.org.uk

Re: [patch] Add insmod option to force the use of the backup timer.

[Gerd Hoffmann]

Dave Jones wrote:
> On Wed, Feb 28, 2007 at 11:23:46AM +0100, Gerd Hoffmann wrote:
>  > The test which automatically enables the backup timer on some HP
>  > machines doesn't trigger on other hardware which needs the backup
>  > timer too.
> 
> Did you figure out *why* that test doesn't trigger?

I didn't, probably a slightly different hardware bug.

> Making that work seems a better solution to me than adding magic
> options that users won't know they have to use.

Sure, that would be better.  I'll leave that to the ibm guys who own the
hardware in question ;)

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: [patch] Add insmod option to force the use of the backup timer.

[Andrew Morton]

On Thu, 01 Mar 2007 14:20:09 +0100 Gerd Hoffmann <kraxel@suse.de> wrote:

> Dave Jones wrote:
> > On Wed, Feb 28, 2007 at 11:23:46AM +0100, Gerd Hoffmann wrote:
> >  > The test which automatically enables the backup timer on some HP
> >  > machines doesn't trigger on other hardware which needs the backup
> >  > timer too.
> > 
> > Did you figure out *why* that test doesn't trigger?
> 
> I didn't, probably a slightly different hardware bug.
> 
> > Making that work seems a better solution to me than adding magic
> > options that users won't know they have to use.
> 
> Sure, that would be better.  I'll leave that to the ibm guys who own the
> hardware in question ;)
> 

Well, it doens't _have_ to be the IBM guys.  Anyone who can reproduce this
problem should be able to find the suitable magic to detect the broken
interrupt generation.

An automatic fix is much preferable to a module parameter which most people
won't even know exists.

Perhaps Alex can suggest some debugging steps we can take to work out
why the test isn't triggering?

Re: [patch] Add insmod option to force the use of the backup timer.

[Alex Williamson]

On Mon, 2007-03-05 at 19:03 -0800, Andrew Morton wrote:
> 
> Perhaps Alex can suggest some debugging steps we can take to work out
> why the test isn't triggering?

   I was lucky and had a hardware description of the bug I was trying to
work around with the 8250 backup timer patch.  If the UART on the system
in question is an integrated component, it might be worthwhile to start
with the errata documented in the hardware manual.

   The detection test is simply looking for UARTs that don't re-assert
the transmit holding register empty interrupt when it becomes
re-enabled.  Since the backup timer is successfully kicking the UART
back into action, it would be interesting to know whether this is
because the "Diva test" is detecting work to do or if it's just a matter
of reading the IIR bits (the interrupt is there, but not getting
delivered).  The state of the UART at that point may be a clue how to
detect the problem.

   UARTs often seem to be the most troubling part of a system, so I'm
not opposed to a boot/module option, but auto-detection provides a much
better user experience.  Thanks,

	Alex

-- 
Alex Williamson                             HP Open Source & Linux Org.