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-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
