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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

+}
+arch_initcall(q40_add_kbd_device);

For the Amiga platform drivers, I used device_initcall().

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.

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

+       kfree(q40kbd);
+
+       platform_set_drvdata(pdev, NULL);

       return 0;
 }

Thanks, it's a nice cleanup!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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


[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux