2008-02-17 21:57:38

by David P. Reed

[permalink] [raw]
Subject: [PATCH 3/3] x86: fix pcspkr to not use inb_p/outb_p calls.

Fix pcspkr driver to use explicit timing delay for access to PIT,
rather than inb_p/outb_p calls, which use insufficiently explicit
delays (defaulting to port 80 writes) that can cause freeze problems
on some machines, such as Quanta moterboard machines using ENE EC's.
The explicit timing delay is only needed for accesses to the 8253 PIT.
The standard requirement for the 8253 to respond to successive writes
is 1 microsecond. The 8253 has never been on the expansion bus, so
a proper delay has nothing to do with expansion bus timing, but instead
its internal logic's capability to react to input. Since udelay is correctly
calibrated by the time the pcspkr driver is initialized, we use 1 microsecond
as the timing.

Also shorten lines to less than 80 characters.

Signed-off-by: David P. Reed <[email protected]>

Index: linux-2.6/drivers/input/misc/pcspkr.c
===================================================================
--- linux-2.6.orig/drivers/input/misc/pcspkr.c
+++ linux-2.6/drivers/input/misc/pcspkr.c
@@ -32,9 +32,11 @@ MODULE_ALIAS("platform:pcspkr");
static DEFINE_SPINLOCK(i8253_lock);
#endif

-static int pcspkr_event(struct input_dev *dev, unsigned int type, unsigned int code, int value)
+static int pcspkr_event(struct input_dev *dev, unsigned int type,
+ unsigned int code, int value)
{
unsigned int count = 0;
+ unsigned char mask;
unsigned long flags;

if (type != EV_SND)
@@ -51,17 +53,21 @@ static int pcspkr_event(struct input_dev

spin_lock_irqsave(&i8253_lock, flags);

+ mask = inb(0x61);
if (count) {
/* enable counter 2 */
- outb_p(inb_p(0x61) | 3, 0x61);
+ outb(mask | 3, 0x61);
+ /* some 8253's may require 1 usec. between accesses */
/* set command for counter 2, 2 byte write */
- outb_p(0xB6, 0x43);
+ outb(0xB6, 0x43);
+ udelay(1);
/* select desired HZ */
- outb_p(count & 0xff, 0x42);
+ outb(count & 0xff, 0x42);
+ udelay(1);
outb((count >> 8) & 0xff, 0x42);
} else {
/* disable counter 2 */
- outb(inb_p(0x61) & 0xFC, 0x61);
+ outb(mask & 0xFC, 0x61);
}

spin_unlock_irqrestore(&i8253_lock, flags);

--


2008-02-17 22:32:38

by Alan

[permalink] [raw]
Subject: Re: [PATCH 3/3] x86: fix pcspkr to not use inb_p/outb_p calls.

On Sun, 17 Feb 2008 16:56:42 -0500 (EST)
"David P. Reed" <[email protected]> wrote:

> Fix pcspkr driver to use explicit timing delay for access to PIT,
> rather than inb_p/outb_p calls, which use insufficiently explicit

NAK.

We now have inb_pit and outb_pit to make the transition easier - can you
switch to those instead of coding internal knowledge outside of the
kernel methods.

They in turn will acquire whatever logic is needed to DTRT.

Alan

2008-02-18 03:12:35

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 3/3] x86: fix pcspkr to not use inb_p/outb_p calls.


* Alan Cox <[email protected]> wrote:

> On Sun, 17 Feb 2008 16:56:42 -0500 (EST)
> "David P. Reed" <[email protected]> wrote:
>
> > Fix pcspkr driver to use explicit timing delay for access to PIT,
> > rather than inb_p/outb_p calls, which use insufficiently explicit
>
> NAK.
>
> We now have inb_pit and outb_pit to make the transition easier - can
> you switch to those instead of coding internal knowledge outside of
> the kernel methods.

please also Cc: Dmitry Torokhov <[email protected]> to
drivers/input/ patches. Once inb_pit/outb_pit is used, as suggested by
Alan, i ack the patch from the arch/x86 perspective.

Ingo