Input: pcspkr - register with driver core as a platfrom device
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/misc/pcspkr.c | 86 ++++++++++++++++++++++++++++++++++++++++----
1 files changed, 80 insertions(+), 6 deletions(-)
Index: work/drivers/input/misc/pcspkr.c
===================================================================
--- work.orig/drivers/input/misc/pcspkr.c
+++ work/drivers/input/misc/pcspkr.c
@@ -16,6 +16,7 @@
#include <linux/module.h>
#include <linux/init.h>
#include <linux/input.h>
+#include <linux/platform_device.h>
#include <asm/8253pit.h>
#include <asm/io.h>
@@ -23,8 +24,7 @@ MODULE_AUTHOR("Vojtech Pavlik <vojtech@u
MODULE_DESCRIPTION("PC Speaker beeper driver");
MODULE_LICENSE("GPL");
-static struct input_dev *pcspkr_dev;
-
+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)
@@ -64,8 +64,11 @@ static int pcspkr_event(struct input_dev
return 0;
}
-static int __init pcspkr_init(void)
+static int __devinit pcspkr_probe(struct platform_device *dev)
{
+ struct input_dev *pcspkr_dev;
+ int err;
+
pcspkr_dev = input_allocate_device();
if (!pcspkr_dev)
return -ENOMEM;
@@ -76,21 +79,92 @@ static int __init pcspkr_init(void)
pcspkr_dev->id.vendor = 0x001f;
pcspkr_dev->id.product = 0x0001;
pcspkr_dev->id.version = 0x0100;
+ pcspkr_dev->cdev.dev = &dev->dev;
pcspkr_dev->evbit[0] = BIT(EV_SND);
pcspkr_dev->sndbit[0] = BIT(SND_BELL) | BIT(SND_TONE);
pcspkr_dev->event = pcspkr_event;
- input_register_device(pcspkr_dev);
+ err = input_register_device(pcspkr_dev);
+ if (err) {
+ input_free_device(pcspkr_dev);
+ return err;
+ }
+
+ platform_set_drvdata(dev, pcspkr_dev);
return 0;
}
-static void __exit pcspkr_exit(void)
+static int __devexit pcspkr_remove(struct platform_device *dev)
{
- input_unregister_device(pcspkr_dev);
+ struct input_dev *pcspkr_dev = platform_get_drvdata(dev);
+
+ input_unregister_device(pcspkr_dev);
+ platform_set_drvdata(dev, NULL);
/* turn off the speaker */
pcspkr_event(NULL, EV_SND, SND_BELL, 0);
+
+ return 0;
+}
+
+static int pcspkr_suspend(struct platform_device *dev, pm_message_t state)
+{
+ pcspkr_event(NULL, EV_SND, SND_BELL, 0);
+
+ return 0;
+}
+
+static void pcspkr_shutdown(struct platform_device *dev)
+{
+ /* turn off the speaker */
+ pcspkr_event(NULL, EV_SND, SND_BELL, 0);
+}
+
+static struct platform_driver pcspkr_platform_driver = {
+ .driver = {
+ .name = "pcspkr",
+ .owner = THIS_MODULE,
+ },
+ .probe = pcspkr_probe,
+ .remove = __devexit_p(pcspkr_remove),
+ .suspend = pcspkr_suspend,
+ .shutdown = pcspkr_shutdown,
+};
+
+
+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;
+}
+
+static void __exit pcspkr_exit(void)
+{
+ platform_device_unregister(pcspkr_platform_device);
+ platform_driver_unregister(&pcspkr_platform_driver);
}
module_init(pcspkr_init);
On Sat, 2006-01-07 at 12:16 -0500, Dmitry Torokhov wrote:
> plain text document attachment (pcspkr-platform-device.patch)
> Input: pcspkr - register with driver core as a platfrom device
Hi Dimitri !
That looks great, something we've been wanting to tackle for a while...
except for one thing :)
The actual creation of the device shouldn't be done there... only the
driver should be there. The device instanciation should be moved to the
i386 arch code (and/or any other architecture that might have this
thing).
On ppc64 for example, we have machines that will blow up when that
driver tries to poke random IOs, but we also have machines that do have
that legacy piece of hardware where expected. We can know it from the
firmware, thus we can decide wether to create the platform device or not
from the arch code.
What do you prefer ? Keep that the way you did for now and add some
#ifdef CONFIG_PPC64 with the ppc64 probe code in that driver or do you
want to call all the way to moving the actual device creation to the
platform code (as I think should be done) ?
Cheers,
Ben.
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
>
> drivers/input/misc/pcspkr.c | 86 ++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 80 insertions(+), 6 deletions(-)
>
> Index: work/drivers/input/misc/pcspkr.c
> ===================================================================
> --- work.orig/drivers/input/misc/pcspkr.c
> +++ work/drivers/input/misc/pcspkr.c
> @@ -16,6 +16,7 @@
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/input.h>
> +#include <linux/platform_device.h>
> #include <asm/8253pit.h>
> #include <asm/io.h>
>
> @@ -23,8 +24,7 @@ MODULE_AUTHOR("Vojtech Pavlik <vojtech@u
> MODULE_DESCRIPTION("PC Speaker beeper driver");
> MODULE_LICENSE("GPL");
>
> -static struct input_dev *pcspkr_dev;
> -
> +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)
> @@ -64,8 +64,11 @@ static int pcspkr_event(struct input_dev
> return 0;
> }
>
> -static int __init pcspkr_init(void)
> +static int __devinit pcspkr_probe(struct platform_device *dev)
> {
> + struct input_dev *pcspkr_dev;
> + int err;
> +
> pcspkr_dev = input_allocate_device();
> if (!pcspkr_dev)
> return -ENOMEM;
> @@ -76,21 +79,92 @@ static int __init pcspkr_init(void)
> pcspkr_dev->id.vendor = 0x001f;
> pcspkr_dev->id.product = 0x0001;
> pcspkr_dev->id.version = 0x0100;
> + pcspkr_dev->cdev.dev = &dev->dev;
>
> pcspkr_dev->evbit[0] = BIT(EV_SND);
> pcspkr_dev->sndbit[0] = BIT(SND_BELL) | BIT(SND_TONE);
> pcspkr_dev->event = pcspkr_event;
>
> - input_register_device(pcspkr_dev);
> + err = input_register_device(pcspkr_dev);
> + if (err) {
> + input_free_device(pcspkr_dev);
> + return err;
> + }
> +
> + platform_set_drvdata(dev, pcspkr_dev);
>
> return 0;
> }
>
> -static void __exit pcspkr_exit(void)
> +static int __devexit pcspkr_remove(struct platform_device *dev)
> {
> - input_unregister_device(pcspkr_dev);
> + struct input_dev *pcspkr_dev = platform_get_drvdata(dev);
> +
> + input_unregister_device(pcspkr_dev);
> + platform_set_drvdata(dev, NULL);
> /* turn off the speaker */
> pcspkr_event(NULL, EV_SND, SND_BELL, 0);
> +
> + return 0;
> +}
> +
> +static int pcspkr_suspend(struct platform_device *dev, pm_message_t state)
> +{
> + pcspkr_event(NULL, EV_SND, SND_BELL, 0);
> +
> + return 0;
> +}
> +
> +static void pcspkr_shutdown(struct platform_device *dev)
> +{
> + /* turn off the speaker */
> + pcspkr_event(NULL, EV_SND, SND_BELL, 0);
> +}
> +
> +static struct platform_driver pcspkr_platform_driver = {
> + .driver = {
> + .name = "pcspkr",
> + .owner = THIS_MODULE,
> + },
> + .probe = pcspkr_probe,
> + .remove = __devexit_p(pcspkr_remove),
> + .suspend = pcspkr_suspend,
> + .shutdown = pcspkr_shutdown,
> +};
> +
> +
> +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;
> +}
> +
> +static void __exit pcspkr_exit(void)
> +{
> + platform_device_unregister(pcspkr_platform_device);
> + platform_driver_unregister(&pcspkr_platform_driver);
> }
>
> module_init(pcspkr_init);
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Tuesday 10 January 2006 01:41, Benjamin Herrenschmidt wrote:
> On Sat, 2006-01-07 at 12:16 -0500, Dmitry Torokhov wrote:
> > plain text document attachment (pcspkr-platform-device.patch)
> > Input: pcspkr - register with driver core as a platfrom device
>
> Hi Dimitri !
>
> That looks great, something we've been wanting to tackle for a while...
> except for one thing :)
>
> The actual creation of the device shouldn't be done there... only the
> driver should be there. The device instanciation should be moved to the
> i386 arch code (and/or any other architecture that might have this
> thing).
>
> On ppc64 for example, we have machines that will blow up when that
> driver tries to poke random IOs, but we also have machines that do have
> that legacy piece of hardware where expected. We can know it from the
> firmware, thus we can decide wether to create the platform device or not
> from the arch code.
>
> What do you prefer ? Keep that the way you did for now and add some
> #ifdef CONFIG_PPC64 with the ppc64 probe code in that driver or do you
> want to call all the way to moving the actual device creation to the
> platform code (as I think should be done) ?
>
Having platform code instantiate platform devices would be great but I
wonder how it will look like on x86 where we don't have a way to enumerate
devices. ACPI might do it but I am not sure if all DSDTs describe beepers...
--
Dmitry
> Having platform code instantiate platform devices would be great but I
> wonder how it will look like on x86 where we don't have a way to enumerate
> devices. ACPI might do it but I am not sure if all DSDTs describe beepers...
I don't know the gory details about x86 so I'll let others decide what
to do there... can't it start by instanciating it unconditionally +/-
maybe a kernel command line to "skip" it in case the hw is really not
legacy ?
Ben.
On Tuesday 10 January 2006 02:04, Benjamin Herrenschmidt wrote:
>
> > Having platform code instantiate platform devices would be great but I
> > wonder how it will look like on x86 where we don't have a way to enumerate
> > devices. ACPI might do it but I am not sure if all DSDTs describe beepers...
>
> I don't know the gory details about x86 so I'll let others decide what
> to do there... can't it start by instanciating it unconditionally +/-
> maybe a kernel command line to "skip" it in case the hw is really not
> legacy ?
>
Maybe.. I need to think about it...
--
Dmitry