2006-02-21 02:18:59

by Michael Neuling

[permalink] [raw]
Subject: [PATCH] input: pcspkr device and driver separation

The current pcspkr code combines the device and driver registration.
This patch splits these, putting the device registration in the arch
specific code.

It seems opinion is divided as how much error checking needs to be
done when registering the device at boot. This patch does a BUG_ON on
any device allocation failures for all architectures touched. The
PowerPC guys are happy with this.

Mikey

Signed-off-by: Michael Neuling <[email protected]>

Split the pcskpr device and driver registration. Adds device
registration for Alpha, MIPS and x86. Also added for PowerPC which is
dependant on the pcspkr existing in the device-tree.
---
arch/alpha/kernel/setup.c | 12 ++++++++++++
arch/i386/kernel/setup.c | 12 ++++++++++++
arch/mips/kernel/setup.c | 12 ++++++++++++
arch/powerpc/kernel/setup-common.c | 18 ++++++++++++++++++
drivers/input/misc/pcspkr.c | 27 +--------------------------
5 files changed, 55 insertions(+), 26 deletions(-)

Index: linux-2.6-linus/arch/i386/kernel/setup.c
===================================================================
--- linux-2.6-linus.orig/arch/i386/kernel/setup.c
+++ linux-2.6-linus/arch/i386/kernel/setup.c
@@ -1630,6 +1630,18 @@ void __init setup_arch(char **cmdline_p)
#endif
}

+static __init int add_pcspkr(void)
+{
+ struct platform_device *pd;
+
+ pd = platform_device_alloc("pcspkr", -1);
+ BUG_ON(!pd);
+ BUG_ON(platform_device_add(pd));
+
+ return 0;
+}
+device_initcall(add_pcspkr);
+
#include "setup_arch_post.h"
/*
* Local Variables:
Index: linux-2.6-linus/arch/powerpc/kernel/setup-common.c
===================================================================
--- linux-2.6-linus.orig/arch/powerpc/kernel/setup-common.c
+++ linux-2.6-linus/arch/powerpc/kernel/setup-common.c
@@ -469,3 +469,21 @@ static int __init early_xmon(char *p)
}
early_param("xmon", early_xmon);
#endif
+
+static __init int add_pcspkr(void)
+{
+ struct device_node *np;
+ struct platform_device *pd;
+
+ np = of_find_compatible_node(NULL, NULL, "pnpPNP,100");
+ of_node_put(np);
+ if (!np)
+ return -ENODEV;
+
+ pd = platform_device_alloc("pcspkr", -1);
+ BUG_ON(!pd);
+ BUG_ON(platform_device_add(pd));
+
+ return 0;
+}
+device_initcall(add_pcspkr);
Index: linux-2.6-linus/drivers/input/misc/pcspkr.c
===================================================================
--- linux-2.6-linus.orig/drivers/input/misc/pcspkr.c
+++ linux-2.6-linus/drivers/input/misc/pcspkr.c
@@ -24,7 +24,6 @@ MODULE_AUTHOR("Vojtech Pavlik <vojtech@u
MODULE_DESCRIPTION("PC Speaker beeper driver");
MODULE_LICENSE("GPL");

-static struct platform_device *pcspkr_platform_device;
static DEFINE_SPINLOCK(i8253_beep_lock);

static int pcspkr_event(struct input_dev *dev, unsigned int type, unsigned int code, int value)
@@ -135,35 +134,11 @@ static struct platform_driver pcspkr_pla

static int __init pcspkr_init(void)
{
- int err;
-
- err = platform_driver_register(&pcspkr_platform_driver);
- if (err)
- return err;
-
- pcspkr_platform_device = platform_device_alloc("pcspkr", -1);
- if (!pcspkr_platform_device) {
- err = -ENOMEM;
- goto err_unregister_driver;
- }
-
- err = platform_device_add(pcspkr_platform_device);
- if (err)
- goto err_free_device;
-
- return 0;
-
- err_free_device:
- platform_device_put(pcspkr_platform_device);
- err_unregister_driver:
- platform_driver_unregister(&pcspkr_platform_driver);
-
- return err;
+ return platform_driver_register(&pcspkr_platform_driver);
}

static void __exit pcspkr_exit(void)
{
- platform_device_unregister(pcspkr_platform_device);
platform_driver_unregister(&pcspkr_platform_driver);
}

Index: linux-2.6-linus/arch/mips/kernel/setup.c
===================================================================
--- linux-2.6-linus.orig/arch/mips/kernel/setup.c
+++ linux-2.6-linus/arch/mips/kernel/setup.c
@@ -559,3 +559,15 @@ int __init dsp_disable(char *s)
}

__setup("nodsp", dsp_disable);
+
+static __init int add_pcspkr(void)
+{
+ struct platform_device *pd;
+
+ pd = platform_device_alloc("pcspkr", -1);
+ BUG_ON(!pd);
+ BUG_ON(platform_device_add(pd));
+
+ return 0;
+}
+device_initcall(add_pcspkr);
Index: linux-2.6-linus/arch/alpha/kernel/setup.c
===================================================================
--- linux-2.6-linus.orig/arch/alpha/kernel/setup.c
+++ linux-2.6-linus/arch/alpha/kernel/setup.c
@@ -1484,3 +1484,15 @@ alpha_panic_event(struct notifier_block
#endif
return NOTIFY_DONE;
}
+
+static __init int add_pcspkr(void)
+{
+ struct platform_device *pd;
+
+ pd = platform_device_alloc("pcspkr", -1);
+ BUGON(!pd);
+ BUGON(platform_device_add(pd));
+
+ return 0;
+}
+device_initcall(add_pcspkr);


2006-02-21 04:24:01

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] input: pcspkr device and driver separation

On Monday 20 February 2006 21:19, Michael Neuling wrote:
> The current pcspkr code combines the device and driver registration.
> This patch splits these, putting the device registration in the arch
> specific code.
>
> It seems opinion is divided as how much error checking needs to be
> done when registering the device at boot. This patch does a BUG_ON on
> any device allocation failures for all architectures touched. The
> PowerPC guys are happy with this.
>


Argh! I mean to write this reply earlier...

As I said I like the patch overall, however I do not agree that BUG_ON is
appropriate here. Whatever issues with speaker device there is they do not
warrant OOPS at boot time. Speaker is not critical device so just clean up
after failure an move on. If you compare code cleaning up with code using
BUG_ON they are about the same in the end.

--
Dmitry