LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [patch] Add insmod option to force the use of the backup timer.
@ 2007-02-28 10:23 Gerd Hoffmann
  2007-02-28 18:55 ` Dave Jones
  0 siblings, 1 reply; 5+ messages in thread
From: Gerd Hoffmann @ 2007-02-28 10:23 UTC (permalink / raw)
  To: linux-kernel; +Cc: Alex Williamson, Kevin Stansell

[-- 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>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] Add insmod option to force the use of the backup timer.
  2007-02-28 10:23 [patch] Add insmod option to force the use of the backup timer Gerd Hoffmann
@ 2007-02-28 18:55 ` Dave Jones
  2007-03-01 13:20   ` Gerd Hoffmann
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Jones @ 2007-02-28 18:55 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: linux-kernel, Alex Williamson, Kevin Stansell

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] Add insmod option to force the use of the backup timer.
  2007-02-28 18:55 ` Dave Jones
@ 2007-03-01 13:20   ` Gerd Hoffmann
  2007-03-06  3:03     ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: Gerd Hoffmann @ 2007-03-01 13:20 UTC (permalink / raw)
  To: Dave Jones, Gerd Hoffmann, linux-kernel, Alex Williamson, Kevin Stansell

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>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] Add insmod option to force the use of the backup timer.
  2007-03-01 13:20   ` Gerd Hoffmann
@ 2007-03-06  3:03     ` Andrew Morton
  2007-03-06  4:22       ` Alex Williamson
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2007-03-06  3:03 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: Dave Jones, linux-kernel, Alex Williamson, Kevin Stansell

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?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] Add insmod option to force the use of the backup timer.
  2007-03-06  3:03     ` Andrew Morton
@ 2007-03-06  4:22       ` Alex Williamson
  0 siblings, 0 replies; 5+ messages in thread
From: Alex Williamson @ 2007-03-06  4:22 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Gerd Hoffmann, Dave Jones, linux-kernel, Kevin Stansell

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.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2007-03-06  4:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-28 10:23 [patch] Add insmod option to force the use of the backup timer Gerd Hoffmann
2007-02-28 18:55 ` Dave Jones
2007-03-01 13:20   ` Gerd Hoffmann
2007-03-06  3:03     ` Andrew Morton
2007-03-06  4:22       ` Alex Williamson

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).