Re: [PATCH] HP Mobile data protection system driver with interrupt handling

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

 




From: "Dmitry Torokhov" <[email protected]>
To: "Andrew Morton" <[email protected]>
CC: "Burman Yan" <[email protected]>, [email protected], "Jean Delvare" <[email protected]> Subject: Re: [PATCH] HP Mobile data protection system driver with interrupt handling
Date: Mon, 6 Nov 2006 17:18:53 -0500

On 11/6/06, Andrew Morton <[email protected]> wrote:
On Fri, 03 Nov 2006 18:33:31 +0200
"Burman Yan" <[email protected]> wrote:
> +
> +static unsigned int mouse = 0;

The `= 0' is unneeded.

> +module_param(mouse, bool, S_IRUGO);
> +MODULE_PARM_DESC(mouse, "Enable the input class device on module load");

Does the parameter have to be called "mouse"? I'd rename it to "input"
and drop the work "class" from parameter description.

Dropping the "class" seems logical, but calling the parameter input
seems confusing to me - to a user that doesn't want to read too much
manual/code and just wants to play around with the device (I do that sometimes)
mouse sounds more reasonable to me.


> +
> +     if (input_register_device(mdps.idev)) {
> +             input_free_device(mdps.idev);
> +             mdps.idev = NULL;
> +             return;
> +     }
> +
> +     mdps.kthread = kthread_run(mdps_mouse_kthread, NULL, "kmdps");
> +     if (IS_ERR(mdps.kthread)) {
> +             input_unregister_device(mdps.idev);
> +             mdps.idev = NULL;
> +             return;
> +     }
> +

Please consider implementing mdps_input_open() and mdps_input_close()
and create and run the polling thread from there - there is no point
in polling the device if noone listens to its events.

You have a point - I'll see into that.


--
Dmitry

_________________________________________________________________
Express yourself instantly with MSN Messenger! Download today it's FREE! http://messenger.msn.click-url.com/go/onm00200471ave/direct/01/

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

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux