Re: [PATCH] Input: q40kbd - convert driver to the split model

On Wed, Jan 11, 2012 at 09:11, Dmitry Torokhov
[off-list ref] wrote:

On Wed, Jan 11, 2012 at 08:55:54AM +0100, Geert Uytterhoeven wrote:

quoted

On Sat, Dec 31, 2011 at 02:11, Dmitry Torokhov
[off-list ref] wrote:

quoted

quoted

+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?

No, m68k does support multi-platform kernels.

quoted

quoted

+}
+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.

So far I haven't encountered any problems on Amiga.
I'll look into this.

quoted

quoted

-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.

And in the probe case that can't happen when request_irq() is called?

Can a call to q40kbd_stop() first fix that?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds