On Thursday 03 February 2005 17:43, Stephen Evanchik wrote:
> Vojtech,
>
> Here is a patch that exposes the IBM TrackPoint's extended properties
> as well as scroll wheel emulation.
>
>
Hi,
Very nice although I have a couple of comments.
> /*
> + * Try to initialize the IBM TrackPoint
> + */
> + if (max_proto > PSMOUSE_PS2 && trackpoint_init(psmouse) == 0) {
> + psmouse->vendor = "IBM";
> + psmouse->name = "TrackPoint";
> +
> + return PSMOUSE_PS2;
Why PSMOUSE_PS2? Reconnect will surely not like it.
> +int tp_sens = TP_DEF_SENS;
> +module_param_named(sens, tp_sens, uint, 0);
> +MODULE_PARM_DESC(sens, "Sensitivity");
....
> +int tp_mb_scroll = TP_DEF_MB_SCROLL;
> +module_param_named(mb_scroll, tp_mb_scroll, uint, 0);
> +MODULE_PARM_DESC(mb_scroll, "Scroll with middle button");
All of this should be handled via sysfs dynamically, we don't need any
more pmouse module parameters.
> +__obsolete_setup("pts=");
> +__obsolete_setup("backup=");
> +__obsolete_setup("draghyst=");
What are these? They can't be obsolete as the module has never been part
of the vanilla kernel.
> +
> +/*
> + * Device IO: read, write and toggle bit
> + */
> +static void trackpoint_command(struct ps2dev *ps2dev, unsigned char cmd)
> +{
> + ps2_command(ps2dev, NULL, MAKE_PS2_CMD(0, 0, TP_COMMAND));
> + ps2_command(ps2dev, NULL, MAKE_PS2_CMD(0, 0, cmd));
> +}
> +
Error checking would be nice.
> +
> +#ifdef CONFIG_PROC_FS
...
> +{
> + remove_proc_entry("scroll_delay", tp_dir);
> + remove_proc_entry("scroll", tp_dir);
> + remove_proc_entry("transparent", tp_dir);
> + remove_proc_entry("ext_dev", tp_dir);
> + remove_proc_entry("mb", tp_dir);
> + remove_proc_entry("skip_back", tp_dir);
> + remove_proc_entry("ptson", tp_dir);
> + remove_proc_entry("jenks_curv", tp_dir);
> + remove_proc_entry("z_time", tp_dir);
> + remove_proc_entry("up_thresh", tp_dir);
> + remove_proc_entry("thresh", tp_dir);
> + remove_proc_entry("min_drag", tp_dir);
> + remove_proc_entry("drag_hyst", tp_dir);
> + remove_proc_entry("backup", tp_dir);
> + remove_proc_entry("neg_inertia", tp_dir);
> + remove_proc_entry("speed", tp_dir);
> + remove_proc_entry("sensitivity", tp_dir);
> + remove_proc_entry("trackpoint", NULL);
> +}
No new proc stuff please (it will be visible from sysfs when you redo the
module parameters).
> +
> + tp->scrolling = 0;
> + tp->mb_was_down = 0;
> + }
> + else if(tp->scrolling) {
> +
> + /* Vertical scrolling */
> + diff = (int) ((packet[0] << 3) & 0x100) - (int) packet[2];
> + if( diff < -2 ) {
> + input_report_rel(&psmouse->dev, REL_WHEEL, 1);
> + }
> + else if(diff > 2) {
> + input_report_rel(&psmouse->dev, REL_WHEEL, -1);
> + }
> +
> + /* Horizontal scrolling */
> + diff = (int) packet[1] - (int) ((packet[0] << 4) & 0x100);
> + if( diff < -2) {
> + input_report_rel(&psmouse->dev, REL_HWHEEL, 1);
> + }
> + else if( diff > 2) {
> + input_report_rel(&psmouse->dev, REL_HWHEEL, -1);
> + }
> +
> + packet[1] &= 0x00;
> + packet[2] &= 0x00;
> + }
Looks like whitespace damage (tabs vs spaces) plus it should be "} else {"
on one line.
> +int trackpoint_reconnect(struct psmouse *psmouse)
> +{
> + unsigned char toggle;
> + struct trackpoint_data *tp = psmouse->private;
> +
> + /* Push the config to the device */
> +
I'd like to verify that it still recognized as trackpoint - suspends often
play tricks on PS/2 devices.
> +
> + printk("IBM TrackPoint firmware: 0x%02X\n", param[1]);
KERN_INFO?
--
Dmitry
-
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/
