INPUT: Sanitize PIT locking in pcspkr

PATCH In review Thomas Gleixner 5 participants 9 messages May 17, 2007 – Jun 12, 2007 Input drivers

lore

Share & tools

lore

Thomas Gleixner

author

2007-05-17 13:31 UTC #0 lore raw

PATCH 1/1 INPUT: Sanitize PIT locking in pcspkr

The PC-speaker code has a quite creative method to serialize access to the PIT: It uses a local lock.

On i386 and x86_64 the access to the PIT is serialized by a lock in the architecture code. The separate locking in the PC-speaker code ignores the global lock and creates a nasty race between the PC-speaker and the PIT clock source / events code on SMP machines.

Use the global i8253_lock instead of the local i8253_beep_lock, when compiled for i386/x86_64.

Signed-off-by Thomas Gleixner <[email protected]>

arch/x86_64/kernel/time.c +2 -0

Index: linux-2.6/arch/x86_64/kernel/time.c
===================================================================
@@ -33,6 +33,7 @@
 #include <acpi/acpi_bus.h>
 #endif
 #include <asm/8253pit.h>
+#include <asm/i8253.h>
 #include <asm/pgtable.h>
 #include <asm/vsyscall.h>
 #include <asm/timex.h>

@@ -50,6 +51,7 @@ static char *timename = NULL;
 DEFINE_SPINLOCK(rtc_lock);
 EXPORT_SYMBOL(rtc_lock);
 DEFINE_SPINLOCK(i8253_lock);
+EXPORT_SYMBOL(i8253_lock);
 
 volatile unsigned long __jiffies __section_jiffies = INITIAL_JIFFIES;

drivers/input/misc/pcspkr.c +8 -3

Index: linux-2.6/drivers/input/misc/pcspkr.c
===================================================================
@@ -24,7 +24,12 @@ MODULE_AUTHOR("Vojtech Pavlik <vojtech@u
 MODULE_DESCRIPTION("PC Speaker beeper driver");
 MODULE_LICENSE("GPL");
 
-static DEFINE_SPINLOCK(i8253_beep_lock);
+#ifdef CONFIG_X86
+/* Use the global PIT lock ! */
+#include <asm/i8253.h>
+#else
+static DEFINE_SPINLOCK(i8253_lock);
+#endif
 
 static int pcspkr_event(struct input_dev *dev, unsigned int type, unsigned int code, int value)
 {

@@ -43,7 +48,7 @@ static int pcspkr_event(struct input_dev
 	if (value > 20 && value < 32767)
 		count = PIT_TICK_RATE / value;
 
-	spin_lock_irqsave(&i8253_beep_lock, flags);
+	spin_lock_irqsave(&i8253_lock, flags);
 
 	if (count) {
 		/* enable counter 2 */

@@ -58,7 +63,7 @@ static int pcspkr_event(struct input_dev
 		outb(inb_p(0x61) & 0xFC, 0x61);
 	}
 
-	spin_unlock_irqrestore(&i8253_beep_lock, flags);
+	spin_unlock_irqrestore(&i8253_lock, flags);
 
 	return 0;
 }

include/asm-x86_64/i8253.h new file +6 -0

Index: linux-2.6/include/asm-x86_64/i8253.h
===================================================================
@@ -0,0 +1,6 @@
+#ifndef __ASM_I8253_H__
+#define __ASM_I8253_H__
+
+extern spinlock_t i8253_lock;
+
+#endif	/* __ASM_I8253_H__ */

Dmitry Torokhov

maintainer

2007-05-17 14:15 UTC #1 lore raw

Hi Thomas,

Quoted 1 line

+#include <asm/i8253.h>

I don't see this file in include/asm-i386 and your patch only creates asm-x86_64...
BTW, is there any reason 8253pit.h can't be used...?

Hmm... The best way IMO woudl be if arch code attached spinlock that should be used by pcspkr driver to pcspkr platform device (as platform_data) and we would not have private lock at all.

BTW, with all that usevent_supress busiess, does pcspkr gets loaded automatically nowadays?

Signature · 1 line

Dmitry

Thomas Gleixner

author

2007-05-17 14:30 UTC #2 lore raw

Dmitry,

Quoted 6 lines · Dmitry Torokhov

Hi Thomas,
Quoted 1 line
+#include <asm/i8253.h>
I don't see this file in include/asm-i386 and your patch only creates asm-x86_64...

[tglx@inhell4 linux-2.6.21]$ ls include/asm-i386/i82* include/asm-i386/i8253.h include/asm-i386/i8259.h

Quoted 1 line

BTW, is there any reason 8253pit.h can't be used...?

The i386 lock is already exported in i8253.h

Quoted 3 lines

Hmm... The best way IMO woudl be if arch code attached spinlock that should be used by pcspkr driver to pcspkr platform device (as platform_data) and we would not have private lock at all.

Sounds ugly, but that's not material for now and cannot be applied to older kernels, which need this fix as well.

BTW, there are more creative PIT users in drivers/input which use the global lock already, but the PIT usage there is definitely broken on anything >= 2.6.21.

Quoted 2 lines

BTW, with all that usevent_supress busiess, does pcspkr gets loaded automatically nowadays?

I have no idea.

tglx

Dmitry Torokhov

maintainer

2007-05-17 14:36 UTC #3 lore raw

Quoted 5 lines · Thomas Gleixner

Dmitry,
Quoted 2 lines · Dmitry Torokhov
Hi Thomas,

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
+#include <asm/i8253.h>
I don't see this file in include/asm-i386 and your patch only creates asm-x86_64...
[tglx@inhell4 linux-2.6.21]$ ls include/asm-i386/i82* include/asm-i386/i8253.h include/asm-i386/i8259.h

Yes, indeed. I wonder how did I manage not to see it...

Quoted 3 lines

Quoted 1 line
BTW, is there any reason 8253pit.h can't be used...?
The i386 lock is already exported in i8253.h

Right, since we do have i8253.h on i386 that question does not make sense.

Quoted 8 lines

Quoted 4 lines
Hmm... The best way IMO woudl be if arch code attached spinlock that should be used by pcspkr driver to pcspkr platform device (as platform_data) and we would not have private lock at all.
Sounds ugly, but that's not material for now and cannot be applied to older kernels, which need this fix as well.

But then the driver code does not care about arch details which is nice IMHO.

Quoted 4 lines

BTW, there are more creative PIT users in drivers/input which use the global lock already, but the PIT usage there is definitely broken on anything >= 2.6.21.

Are you talking about drivers/input/joystick/analog.c? What is broken there?

Signature · 1 line

Dmitry

Thomas Gleixner

author

2007-05-17 14:45 UTC #4 lore raw

Quoted 6 lines · Dmitry Torokhov

Quoted 4 lines
BTW, there are more creative PIT users in drivers/input which use the global lock already, but the PIT usage there is definitely broken on anything >= 2.6.21.
Are you talking about drivers/input/joystick/analog.c? What is broken there?

drivers/gameport/gameport.c as well.

Both read the PIT directly, which will lead to interesting results. The PIT is either stopped or it can be used in one shot mode with per event changing intervals due to the changes introduced by the clock events layer.

This code should use ktime_get() and not fiddle in the hardware itself.

tglx

Andi Kleen

2007-05-18 04:28 UTC #5 lore raw

Quoted 6 lines · Thomas Gleixner

Both read the PIT directly, which will lead to interesting results. The PIT is either stopped or it can be used in one shot mode with per event changing intervals due to the changes introduced by the clock events layer.
This code should use ktime_get() and not fiddle in the hardware itself.

The gameport code also does TSC accesses directly which is similarly broken. I remember complaining about that some time ago, but it never got fixed in the end.

-Andi

Dmitry Torokhov

2007-05-18 06:27 UTC #6 lore raw

In reply to Thomas Gleixner

Quoted 8 lines · Thomas Gleixner

Quoted 6 lines · Dmitry Torokhov
Quoted 4 lines
BTW, there are more creative PIT users in drivers/input which use the global lock already, but the PIT usage there is definitely broken on anything >= 2.6.21.
Are you talking about drivers/input/joystick/analog.c? What is broken there?

Quoted 8 lines

drivers/gameport/gameport.c as well.
Both read the PIT directly, which will lead to interesting results. The PIT is either stopped or it can be used in one shot mode with per event changing intervals due to the changes introduced by the clock events layer.
This code should use ktime_get() and not fiddle in the hardware itself.

So you are saying that gameport should depend on hrtimers? OK...

And now that I can post properly without attachments here is the patch that attaches spinlock to platform device for pcspkr driver to use.

Signature · 355 lines

Dmitry

Input: pcspkr - use proper lock

On i386 and x86_64 the access to the PIT is serialized by a lock
in the architecture code. The separate locking in the PC-speaker
code ignores the global lock and creates a nasty race between the
PC-speaker and the PIT clock source/events code on SMP machines.

To fix this architecture code attaches proper lock to the
pcspkr platform device and the driver uses it instead of it's
own private lock.

Noticed by Thomas Gleixner <[email protected]>

Also resore uevent generation for pcspkr devices so that the
driver can be loaded automatically by udev.

Signed-off-by: Dmitry Torokhov <[email protected]>
---

 arch/i386/kernel/pcspeaker.c       |   20 -------------------
 arch/alpha/kernel/setup.c          |    9 +++++++-
 arch/i386/kernel/Makefile          |    1 
 arch/i386/kernel/i8253.c           |   23 ++++++++++++++++++++++
 arch/mips/kernel/i8253.c           |    9 +++++++-
 arch/powerpc/kernel/setup-common.c |    9 +++++++-
 arch/x86_64/kernel/Makefile        |    2 -
 arch/x86_64/kernel/time.c          |   38 +++++++++++++++++++++++++++++--------
 drivers/input/misc/pcspkr.c        |   10 +++++----
 9 files changed, 83 insertions(+), 38 deletions(-)

Index: work/arch/alpha/kernel/setup.c
===================================================================
--- work.orig/arch/alpha/kernel/setup.c
+++ work/arch/alpha/kernel/setup.c
@@ -1491,6 +1491,8 @@ alpha_panic_event(struct notifier_block 
         return NOTIFY_DONE;
 }
 
+static DEFINE_SPINLOCK(i8253_lock);
+
 static __init int add_pcspkr(void)
 {
 	struct platform_device *pd;
@@ -1500,9 +1502,14 @@ static __init int add_pcspkr(void)
 	if (!pd)
 		return -ENOMEM;
 
+	pd->dev.platform_data = &i8253_lock;
+	pd->dev.uevent_suppress = 0;
+
 	ret = platform_device_add(pd);
-	if (ret)
+	if (ret) {
+		pd->dev.platform_data = NULL;	/* so we don't try to free it */
 		platform_device_put(pd);
+	}
 
 	return ret;
 }
Index: work/arch/mips/kernel/i8253.c
===================================================================
--- work.orig/arch/mips/kernel/i8253.c
+++ work/arch/mips/kernel/i8253.c
@@ -10,6 +10,8 @@
 
 #include <linux/platform_device.h>
 
+static DEFINE_SPINLOCK(i8253_lock);
+
 static __init int add_pcspkr(void)
 {
 	struct platform_device *pd;
@@ -19,9 +21,14 @@ static __init int add_pcspkr(void)
 	if (!pd)
 		return -ENOMEM;
 
+	pd->dev.platform_data = &i8253_lock;
+	pd->dev.uevent_suppress = 0;
+
 	ret = platform_device_add(pd);
-	if (ret)
+	if (ret) {
+		pd->dev.platform_data = NULL;	/* so we don't try to free it */
 		platform_device_put(pd);
+	}
 
 	return ret;
 }
Index: work/arch/x86_64/kernel/time.c
===================================================================
--- work.orig/arch/x86_64/kernel/time.c
+++ work/arch/x86_64/kernel/time.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/sysdev.h>
+#include <linux/platform_device.h>
 #include <linux/bcd.h>
 #include <linux/notifier.h>
 #include <linux/cpu.h>
@@ -185,7 +186,7 @@ void main_timer_handler(void)
 		set_rtc_mmss(xtime.tv_sec);
 		rtc_update = xtime.tv_sec + 660;
 	}
- 
+
 	write_sequnlock(&xtime_lock);
 }
 
@@ -226,7 +227,7 @@ static unsigned long get_cmos_time(void)
 	/*
 	 * We know that x86-64 always uses BCD format, no need to check the
 	 * config register.
- 	 */
+	 */
 
 	BCD_TO_BIN(sec);
 	BCD_TO_BIN(min);
@@ -239,11 +240,11 @@ static unsigned long get_cmos_time(void)
 		BCD_TO_BIN(century);
 		year += century * 100;
 		printk(KERN_INFO "Extended CMOS year: %d\n", century * 100);
-	} else { 
+	} else {
 		/*
 		 * x86-64 systems only exists since 2002.
 		 * This will work up to Dec 31, 2100
-	 	 */
+		 */
 		year += 2000;
 	}
 
@@ -321,7 +322,7 @@ static unsigned int __init pit_calibrate
 	end = get_cycles_sync();
 
 	spin_unlock_irqrestore(&i8253_lock, flags);
-	
+
 	return (end - start) / 50;
 }
 
@@ -366,7 +367,7 @@ static struct irqaction irq0 = {
 	.handler	= timer_interrupt,
 	.flags		= IRQF_DISABLED | IRQF_IRQPOLL,
 	.mask		= CPU_MASK_NONE,
-	.name 		= "timer"
+	.name		= "timer"
 };
 
 void __init time_init(void)
@@ -384,7 +385,7 @@ void __init time_init(void)
 
 	if (hpet_use_timer) {
 		/* set tick_nsec to use the proper rate for HPET */
-	  	tick_nsec = TICK_NSEC_HPET;
+		tick_nsec = TICK_NSEC_HPET;
 		tsc_khz = hpet_calibrate_tsc();
 		timename = "HPET";
 	} else {
@@ -485,5 +486,26 @@ static int time_init_device(void)
 		error = sysdev_register(&device_timer);
 	return error;
 }
-
 device_initcall(time_init_device);
+
+static __init int add_pcspkr(void)
+{
+	struct platform_device *pd;
+	int ret;
+
+	pd = platform_device_alloc("pcspkr", -1);
+	if (!pd)
+		return -ENOMEM;
+
+	pd->dev.platform_data = &i8253_lock;
+	pd->dev.uevent_suppress = 0;
+
+	ret = platform_device_add(pd);
+	if (ret) {
+		pd->dev.platform_data = NULL;	/* so we don't try to free it */
+		platform_device_put(pd);
+	}
+
+	return ret;
+}
+device_initcall(add_pcspkr);
Index: work/arch/i386/kernel/i8253.c
===================================================================
--- work.orig/arch/i386/kernel/i8253.c
+++ work/arch/i386/kernel/i8253.c
@@ -6,6 +6,7 @@
 #include <linux/spinlock.h>
 #include <linux/jiffies.h>
 #include <linux/sysdev.h>
+#include <linux/platform_device.h>
 #include <linux/module.h>
 #include <linux/init.h>
 
@@ -204,3 +205,25 @@ static int __init init_pit_clocksource(v
 	return clocksource_register(&clocksource_pit);
 }
 arch_initcall(init_pit_clocksource);
+
+static __init int add_pcspkr(void)
+{
+	struct platform_device *pd;
+	int ret;
+
+	pd = platform_device_alloc("pcspkr", -1);
+	if (!pd)
+		return -ENOMEM;
+
+	pd->dev.platform_data = &i8253_lock;
+	pd->dev.uevent_suppress = 0;
+
+	ret = platform_device_add(pd);
+	if (ret) {
+		pd->dev.platform_data = NULL;	/* so we don't try to free it */
+		platform_device_put(pd);
+	}
+
+	return ret;
+}
+device_initcall(add_pcspkr);
Index: work/arch/x86_64/kernel/Makefile
===================================================================
--- work.orig/arch/x86_64/kernel/Makefile
+++ work/arch/x86_64/kernel/Makefile
@@ -44,7 +44,6 @@ obj-$(CONFIG_PCI)		+= early-quirks.o
 
 obj-y				+= topology.o
 obj-y				+= intel_cacheinfo.o
-obj-y				+= pcspeaker.o
 
 CFLAGS_vsyscall.o		:= $(PROFILING) -g0
 
@@ -59,5 +58,4 @@ quirks-y			+= ../../i386/kernel/quirks.o
 i8237-y				+= ../../i386/kernel/i8237.o
 msr-$(subst m,y,$(CONFIG_X86_MSR))  += ../../i386/kernel/msr.o
 alternative-y			+= ../../i386/kernel/alternative.o
-pcspeaker-y			+= ../../i386/kernel/pcspeaker.o
 perfctr-watchdog-y		+= ../../i386/kernel/cpu/perfctr-watchdog.o
Index: work/arch/i386/kernel/Makefile
===================================================================
--- work.orig/arch/i386/kernel/Makefile
+++ work/arch/i386/kernel/Makefile
@@ -43,7 +43,6 @@ obj-$(CONFIG_K8_NB)		+= k8.o
 
 obj-$(CONFIG_VMI)		+= vmi.o vmiclock.o
 obj-$(CONFIG_PARAVIRT)		+= paravirt.o
-obj-y				+= pcspeaker.o
 
 obj-$(CONFIG_SCx200)		+= scx200.o
 
Index: work/arch/i386/kernel/pcspeaker.c
===================================================================
--- work.orig/arch/i386/kernel/pcspeaker.c
+++ /dev/null
@@ -1,20 +0,0 @@
-#include <linux/platform_device.h>
-#include <linux/errno.h>
-#include <linux/init.h>
-
-static __init int add_pcspkr(void)
-{
-	struct platform_device *pd;
-	int ret;
-
-	pd = platform_device_alloc("pcspkr", -1);
-	if (!pd)
-		return -ENOMEM;
-
-	ret = platform_device_add(pd);
-	if (ret)
-		platform_device_put(pd);
-
-	return ret;
-}
-device_initcall(add_pcspkr);
Index: work/drivers/input/misc/pcspkr.c
===================================================================
--- work.orig/drivers/input/misc/pcspkr.c
+++ work/drivers/input/misc/pcspkr.c
@@ -24,10 +24,10 @@ MODULE_AUTHOR("Vojtech Pavlik <vojtech@u
 MODULE_DESCRIPTION("PC Speaker beeper driver");
 MODULE_LICENSE("GPL");
 
-static DEFINE_SPINLOCK(i8253_beep_lock);
-
 static int pcspkr_event(struct input_dev *dev, unsigned int type, unsigned int code, int value)
 {
+	struct platform_device *pdev = input_get_drvdata(dev);
+	spinlock_t *i8253_lock = pdev->dev.platform_data;
 	unsigned int count = 0;
 	unsigned long flags;
 
@@ -43,7 +43,7 @@ static int pcspkr_event(struct input_dev
 	if (value > 20 && value < 32767)
 		count = PIT_TICK_RATE / value;
 
-	spin_lock_irqsave(&i8253_beep_lock, flags);
+	spin_lock_irqsave(i8253_lock, flags);
 
 	if (count) {
 		/* enable counter 2 */
@@ -58,7 +58,7 @@ static int pcspkr_event(struct input_dev
 		outb(inb_p(0x61) & 0xFC, 0x61);
 	}
 
-	spin_unlock_irqrestore(&i8253_beep_lock, flags);
+	spin_unlock_irqrestore(i8253_lock, flags);
 
 	return 0;
 }
@@ -84,6 +84,8 @@ static int __devinit pcspkr_probe(struct
 	pcspkr_dev->sndbit[0] = BIT(SND_BELL) | BIT(SND_TONE);
 	pcspkr_dev->event = pcspkr_event;
 
+	input_set_drvdata(pcspkr_dev, dev);
+
 	err = input_register_device(pcspkr_dev);
 	if (err) {
 		input_free_device(pcspkr_dev);
Index: work/arch/powerpc/kernel/setup-common.c
===================================================================
--- work.orig/arch/powerpc/kernel/setup-common.c
+++ work/arch/powerpc/kernel/setup-common.c
@@ -424,6 +424,8 @@ void __init smp_setup_cpu_maps(void)
 }
 #endif /* CONFIG_SMP */
 
+static DEFINE_SPINLOCK(i8253_lock);
+
 static __init int add_pcspkr(void)
 {
 	struct device_node *np;
@@ -439,9 +441,14 @@ static __init int add_pcspkr(void)
 	if (!pd)
 		return -ENOMEM;
 
+	pd->dev.platform_data = &i8253_lock;
+	pd->dev.uevent_suppress = 0;
+
 	ret = platform_device_add(pd);
-	if (ret)
+	if (ret) {
+		pd->dev.platform_data = NULL;	/* so we don't try to free it */
 		platform_device_put(pd);
+	}
 
 	return ret;
 }

Chris Wright

2007-06-12 00:29 UTC #7 lore raw

In reply to Thomas Gleixner

Quoted 10 lines · * Thomas Gleixner ([email protected]) wrote:

The PC-speaker code has a quite creative method to serialize access to the PIT: It uses a local lock.
On i386 and x86_64 the access to the PIT is serialized by a lock in the architecture code. The separate locking in the PC-speaker code ignores the global lock and creates a nasty race between the PC-speaker and the PIT clock source / events code on SMP machines.
Use the global i8253_lock instead of the local i8253_beep_lock, when compiled for i386/x86_64.

Seems this one got lost?

thanks,
-chris

Dmitry Torokhov

2007-06-12 05:50 UTC #8 lore raw

Quoted 9 lines · Chris Wright

Quoted 8 lines · * Thomas Gleixner ([email protected]) wrote:
The PC-speaker code has a quite creative method to serialize access to the PIT: It uses a local lock.
On i386 and x86_64 the access to the PIT is serialized by a lock in the architecture code. The separate locking in the PC-speaker code ignores the global lock and creates a nasty race between the PC-speaker and the PIT clock source / events code on SMP machines.

Quoted 5 lines

Quoted 2 lines
Use the global i8253_lock instead of the local i8253_beep_lock, when compiled for i386/x86_64.
Seems this one got lost?

Yep... Here is the patch I'd like to get in 2.6.22 but as it touches couple of arches I can't test on I am a bit hesitant to push it through my tree. Thomas OKed it but nobody else responded.

Signature · 355 lines

Dmitry

Input: pcspkr - use proper lock

On i386 and x86_64 the access to the PIT is serialized by a lock
in the architecture code. The separate locking in the PC-speaker
code ignores the global lock and creates a nasty race between the
PC-speaker and the PIT clock source/events code on SMP machines.

To fix this we architecture code attaches proper lock to the
pcspkr platform device and the driver uses it instead of it's
own private lock.

Noticed by Thomas Gleixner <[email protected]>

Also resore uevent generation for pcspkr devices so that the
driver can be loaded automatically by udev.

Signed-off-by: Dmitry Torokhov <[email protected]>
---

 arch/i386/kernel/pcspeaker.c       |   20 -------------------
 arch/alpha/kernel/setup.c          |    9 +++++++-
 arch/i386/kernel/Makefile          |    1 
 arch/i386/kernel/i8253.c           |   23 ++++++++++++++++++++++
 arch/mips/kernel/i8253.c           |    9 +++++++-
 arch/powerpc/kernel/setup-common.c |    9 +++++++-
 arch/x86_64/kernel/Makefile        |    2 -
 arch/x86_64/kernel/time.c          |   38 +++++++++++++++++++++++++++++--------
 drivers/input/misc/pcspkr.c        |   10 +++++----
 9 files changed, 83 insertions(+), 38 deletions(-)

Index: work/arch/alpha/kernel/setup.c
===================================================================
--- work.orig/arch/alpha/kernel/setup.c
+++ work/arch/alpha/kernel/setup.c
@@ -1491,6 +1491,8 @@ alpha_panic_event(struct notifier_block 
         return NOTIFY_DONE;
 }
 
+static DEFINE_SPINLOCK(i8253_lock);
+
 static __init int add_pcspkr(void)
 {
 	struct platform_device *pd;
@@ -1500,9 +1502,14 @@ static __init int add_pcspkr(void)
 	if (!pd)
 		return -ENOMEM;
 
+	pd->dev.platform_data = &i8253_lock;
+	pd->dev.uevent_suppress = 0;
+
 	ret = platform_device_add(pd);
-	if (ret)
+	if (ret) {
+		pd->dev.platform_data = NULL;	/* so we don't try to free it */
 		platform_device_put(pd);
+	}
 
 	return ret;
 }
Index: work/arch/mips/kernel/i8253.c
===================================================================
--- work.orig/arch/mips/kernel/i8253.c
+++ work/arch/mips/kernel/i8253.c
@@ -10,6 +10,8 @@
 
 #include <linux/platform_device.h>
 
+static DEFINE_SPINLOCK(i8253_lock);
+
 static __init int add_pcspkr(void)
 {
 	struct platform_device *pd;
@@ -19,9 +21,14 @@ static __init int add_pcspkr(void)
 	if (!pd)
 		return -ENOMEM;
 
+	pd->dev.platform_data = &i8253_lock;
+	pd->dev.uevent_suppress = 0;
+
 	ret = platform_device_add(pd);
-	if (ret)
+	if (ret) {
+		pd->dev.platform_data = NULL;	/* so we don't try to free it */
 		platform_device_put(pd);
+	}
 
 	return ret;
 }
Index: work/arch/x86_64/kernel/time.c
===================================================================
--- work.orig/arch/x86_64/kernel/time.c
+++ work/arch/x86_64/kernel/time.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/sysdev.h>
+#include <linux/platform_device.h>
 #include <linux/bcd.h>
 #include <linux/notifier.h>
 #include <linux/cpu.h>
@@ -185,7 +186,7 @@ void main_timer_handler(void)
 		set_rtc_mmss(xtime.tv_sec);
 		rtc_update = xtime.tv_sec + 660;
 	}
- 
+
 	write_sequnlock(&xtime_lock);
 }
 
@@ -226,7 +227,7 @@ static unsigned long get_cmos_time(void)
 	/*
 	 * We know that x86-64 always uses BCD format, no need to check the
 	 * config register.
- 	 */
+	 */
 
 	BCD_TO_BIN(sec);
 	BCD_TO_BIN(min);
@@ -239,11 +240,11 @@ static unsigned long get_cmos_time(void)
 		BCD_TO_BIN(century);
 		year += century * 100;
 		printk(KERN_INFO "Extended CMOS year: %d\n", century * 100);
-	} else { 
+	} else {
 		/*
 		 * x86-64 systems only exists since 2002.
 		 * This will work up to Dec 31, 2100
-	 	 */
+		 */
 		year += 2000;
 	}
 
@@ -321,7 +322,7 @@ static unsigned int __init pit_calibrate
 	end = get_cycles_sync();
 
 	spin_unlock_irqrestore(&i8253_lock, flags);
-	
+
 	return (end - start) / 50;
 }
 
@@ -366,7 +367,7 @@ static struct irqaction irq0 = {
 	.handler	= timer_interrupt,
 	.flags		= IRQF_DISABLED | IRQF_IRQPOLL,
 	.mask		= CPU_MASK_NONE,
-	.name 		= "timer"
+	.name		= "timer"
 };
 
 void __init time_init(void)
@@ -384,7 +385,7 @@ void __init time_init(void)
 
 	if (hpet_use_timer) {
 		/* set tick_nsec to use the proper rate for HPET */
-	  	tick_nsec = TICK_NSEC_HPET;
+		tick_nsec = TICK_NSEC_HPET;
 		tsc_khz = hpet_calibrate_tsc();
 		timename = "HPET";
 	} else {
@@ -485,5 +486,26 @@ static int time_init_device(void)
 		error = sysdev_register(&device_timer);
 	return error;
 }
-
 device_initcall(time_init_device);
+
+static __init int add_pcspkr(void)
+{
+	struct platform_device *pd;
+	int ret;
+
+	pd = platform_device_alloc("pcspkr", -1);
+	if (!pd)
+		return -ENOMEM;
+
+	pd->dev.platform_data = &i8253_lock;
+	pd->dev.uevent_suppress = 0;
+
+	ret = platform_device_add(pd);
+	if (ret) {
+		pd->dev.platform_data = NULL;	/* so we don't try to free it */
+		platform_device_put(pd);
+	}
+
+	return ret;
+}
+device_initcall(add_pcspkr);
Index: work/arch/i386/kernel/i8253.c
===================================================================
--- work.orig/arch/i386/kernel/i8253.c
+++ work/arch/i386/kernel/i8253.c
@@ -6,6 +6,7 @@
 #include <linux/spinlock.h>
 #include <linux/jiffies.h>
 #include <linux/sysdev.h>
+#include <linux/platform_device.h>
 #include <linux/module.h>
 #include <linux/init.h>
 
@@ -204,3 +205,25 @@ static int __init init_pit_clocksource(v
 	return clocksource_register(&clocksource_pit);
 }
 arch_initcall(init_pit_clocksource);
+
+static __init int add_pcspkr(void)
+{
+	struct platform_device *pd;
+	int ret;
+
+	pd = platform_device_alloc("pcspkr", -1);
+	if (!pd)
+		return -ENOMEM;
+
+	pd->dev.platform_data = &i8253_lock;
+	pd->dev.uevent_suppress = 0;
+
+	ret = platform_device_add(pd);
+	if (ret) {
+		pd->dev.platform_data = NULL;	/* so we don't try to free it */
+		platform_device_put(pd);
+	}
+
+	return ret;
+}
+device_initcall(add_pcspkr);
Index: work/arch/x86_64/kernel/Makefile
===================================================================
--- work.orig/arch/x86_64/kernel/Makefile
+++ work/arch/x86_64/kernel/Makefile
@@ -44,7 +44,6 @@ obj-$(CONFIG_PCI)		+= early-quirks.o
 
 obj-y				+= topology.o
 obj-y				+= intel_cacheinfo.o
-obj-y				+= pcspeaker.o
 
 CFLAGS_vsyscall.o		:= $(PROFILING) -g0
 
@@ -59,5 +58,4 @@ quirks-y			+= ../../i386/kernel/quirks.o
 i8237-y				+= ../../i386/kernel/i8237.o
 msr-$(subst m,y,$(CONFIG_X86_MSR))  += ../../i386/kernel/msr.o
 alternative-y			+= ../../i386/kernel/alternative.o
-pcspeaker-y			+= ../../i386/kernel/pcspeaker.o
 perfctr-watchdog-y		+= ../../i386/kernel/cpu/perfctr-watchdog.o
Index: work/arch/i386/kernel/Makefile
===================================================================
--- work.orig/arch/i386/kernel/Makefile
+++ work/arch/i386/kernel/Makefile
@@ -43,7 +43,6 @@ obj-$(CONFIG_K8_NB)		+= k8.o
 
 obj-$(CONFIG_VMI)		+= vmi.o vmiclock.o
 obj-$(CONFIG_PARAVIRT)		+= paravirt.o
-obj-y				+= pcspeaker.o
 
 obj-$(CONFIG_SCx200)		+= scx200.o
 
Index: work/arch/i386/kernel/pcspeaker.c
===================================================================
--- work.orig/arch/i386/kernel/pcspeaker.c
+++ /dev/null
@@ -1,20 +0,0 @@
-#include <linux/platform_device.h>
-#include <linux/errno.h>
-#include <linux/init.h>
-
-static __init int add_pcspkr(void)
-{
-	struct platform_device *pd;
-	int ret;
-
-	pd = platform_device_alloc("pcspkr", -1);
-	if (!pd)
-		return -ENOMEM;
-
-	ret = platform_device_add(pd);
-	if (ret)
-		platform_device_put(pd);
-
-	return ret;
-}
-device_initcall(add_pcspkr);
Index: work/drivers/input/misc/pcspkr.c
===================================================================
--- work.orig/drivers/input/misc/pcspkr.c
+++ work/drivers/input/misc/pcspkr.c
@@ -24,10 +24,10 @@ MODULE_AUTHOR("Vojtech Pavlik <vojtech@u
 MODULE_DESCRIPTION("PC Speaker beeper driver");
 MODULE_LICENSE("GPL");
 
-static DEFINE_SPINLOCK(i8253_beep_lock);
-
 static int pcspkr_event(struct input_dev *dev, unsigned int type, unsigned int code, int value)
 {
+	struct platform_device *pdev = input_get_drvdata(dev);
+	spinlock_t *i8253_lock = pdev->dev.platform_data;
 	unsigned int count = 0;
 	unsigned long flags;
 
@@ -43,7 +43,7 @@ static int pcspkr_event(struct input_dev
 	if (value > 20 && value < 32767)
 		count = PIT_TICK_RATE / value;
 
-	spin_lock_irqsave(&i8253_beep_lock, flags);
+	spin_lock_irqsave(i8253_lock, flags);
 
 	if (count) {
 		/* enable counter 2 */
@@ -58,7 +58,7 @@ static int pcspkr_event(struct input_dev
 		outb(inb_p(0x61) & 0xFC, 0x61);
 	}
 
-	spin_unlock_irqrestore(&i8253_beep_lock, flags);
+	spin_unlock_irqrestore(i8253_lock, flags);
 
 	return 0;
 }
@@ -84,6 +84,8 @@ static int __devinit pcspkr_probe(struct
 	pcspkr_dev->sndbit[0] = BIT(SND_BELL) | BIT(SND_TONE);
 	pcspkr_dev->event = pcspkr_event;
 
+	input_set_drvdata(pcspkr_dev, dev);
+
 	err = input_register_device(pcspkr_dev);
 	if (err) {
 		input_free_device(pcspkr_dev);
Index: work/arch/powerpc/kernel/setup-common.c
===================================================================
--- work.orig/arch/powerpc/kernel/setup-common.c
+++ work/arch/powerpc/kernel/setup-common.c
@@ -424,6 +424,8 @@ void __init smp_setup_cpu_maps(void)
 }
 #endif /* CONFIG_SMP */
 
+static DEFINE_SPINLOCK(i8253_lock);
+
 static __init int add_pcspkr(void)
 {
 	struct device_node *np;
@@ -439,9 +441,14 @@ static __init int add_pcspkr(void)
 	if (!pd)
 		return -ENOMEM;
 
+	pd->dev.platform_data = &i8253_lock;
+	pd->dev.uevent_suppress = 0;
+
 	ret = platform_device_add(pd);
-	if (ret)
+	if (ret) {
+		pd->dev.platform_data = NULL;	/* so we don't try to free it */
 		platform_device_put(pd);
+	}
 
 	return ret;
 }