On Wed, Jan 11, 2012 at 08:55:54AM +0100, Geert Uytterhoeven wrote:
Hi Dmitry,
On Sat, Dec 31, 2011 at 02:11, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
Convert the driver to standard spilt model arch-specific code registers
platform device to which driver code can bind later.
Also request IRQ immediately upon binding to the device instead of doing
this when serio port is being opened.
Signed-off-by: Dmitry Torokhov <dtor@xxxxxxx>
---
Not tested as I don't have the hardware...
Unfortunately I also don't have the hardware.
But it compiles. A few comments below.
diff --git a/arch/m68k/q40/config.c b/arch/m68k/q40/config.c
index ad10fec..27d3f76 100644
--- a/arch/m68k/q40/config.c
+++ b/arch/m68k/q40/config.c
+static struct platform_device q40_kbd_pdev = {
+ .name = "q40kbd",
+ .id = -1,
+};
+
+static __init int q40_add_kbd_device(void)
+{
+ return platform_device_register(&q40_kbd_pdev);
If you would use platform_device_register_simple(), you don't need the
q40_kbd_pdev above, reducing memory consumption on non-Q40 platforms.
Isn't this file only compiled on q40 platforms?
+}
+arch_initcall(q40_add_kbd_device);
For the Amiga platform drivers, I used device_initcall().
Won't it potentially race with initialization of q40kbd? It looks like
module_initcall is the same as device_initcall() when module is compiled
in. Given that q40kbd uses platform_dveice_probe() losing race might be
fatal.
diff --git a/drivers/input/serio/q40kbd.c b/drivers/input/serio/q40kbd.c
index 5eb84b3..1e61dd4 100644
--- a/drivers/input/serio/q40kbd.c
+++ b/drivers/input/serio/q40kbd.c
+static void q40kbd_stop(struct q40kbd *q40kbd)
+{
The q40kbd parameters is unused and can be removed.
-static int __devinit q40kbd_probe(struct platform_device *dev)
+static int __devinit q40kbd_probe(struct platform_device *pdev)
+ serio_register_port(q40kbd->port);
This cannot fail?
I see it returns void, but serio_queue_event() inside can fail and thus
won't propagate its error condition and code.
Yes, this is current limitation of serio.
-static int __devexit q40kbd_remove(struct platform_device *dev)
+static int __devexit q40kbd_remove(struct platform_device *pdev)
{
- serio_unregister_port(q40kbd_port);
+ struct q40kbd *q40kbd = platform_get_drvdata(pdev);
+
+ free_irq(Q40_IRQ_KEYBOARD, q40kbd);
+
+ serio_unregister_port(q40kbd->port);
Should the unregister be done before freeing the IRQ, i.e. reverse
order compared to probe?
Unregister will most likely cause memory being freed; you don't want to
chance IRQ firing here.
+ kfree(q40kbd);
+
+ platform_set_drvdata(pdev, NULL);
return 0;
}
Thanks, it's a nice cleanup!
Thanks for lookign this over.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
