linux-kernel - Re: [PATCH 5/7] HID: N-trig MTM Driver fix And cleanup patch 5

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4B95A477.7060104@seas.upenn.edu>
Date:	Mon, 08 Mar 2010 20:29:27 -0500
From:	Rafi Rubin <rafi@...s.upenn.edu>
To:	mickib1@...il.com
CC:	jkosina@...e.cz, chatty@...c.fr, peterhuewe@....de,
	micki@...rig.com, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org, rafi@...s.upenn.edu
Subject: Re: [PATCH 5/7] HID: N-trig MTM Driver fix And cleanup patch 5

> -	if (id->driver_data)
> -		hdev->quirks |= HID_QUIRK_MULTI_INPUT;

This undoes the unfortunate merger of the pen and touch events.  Why 
would you want to remove that?  Is there any actual reason aside from 
that's the way you've done it so far?

I admit that if the user space is smart enough to distinguish the 
multiplexed streams, its not that big a deal.  But even then it 
complicates the user space and adds a few assumptions.

Also, splitting them gives the user the freedom to use different drivers 
for the different sensors.  I understand why you'd want to encourage 
users to use your stack for everything, but that also means you are 
potentially increasing your own support burden.

All in all, it just seems like a bad idea to me.
> -	list_for_each_entry(hidinput,&hdev->inputs, list) {
> -		input = hidinput->input;
> -		switch (hidinput->report->field[0]->application) {
> -		case HID_DG_PEN:
> -			input->name = "N-Trig Pen";
> -			break;
> -		case HID_DG_TOUCHSCREEN:
> -			__clear_bit(BTN_TOOL_PEN, input->keybit);
> -			/*
> -			 * A little something special to enable
> -			 * two and three finger taps.
> -			 */
> -			__set_bit(BTN_TOOL_DOUBLETAP, input->keybit);
> -			__set_bit(BTN_TOOL_TRIPLETAP, input->keybit);
> -			__set_bit(BTN_TOOL_QUADTAP, input->keybit);
> -			/*
> -			 * The physical touchscreen (single touch)
> -			 * input has a value for physical, whereas
> -			 * the multitouch only has logical input
> -			 * fields.
> -			 */
> -			input->name =
> -				(hidinput->report->field[0]
> -				 ->physical) ?
> -				"N-Trig Touchscreen" :
> -				"N-Trig MultiTouch";
> -			break;
> -		}

Even if you want to argue that you need to pull the multi-input quirk, 
that doesn't justify pulling the names.  Perhaps you may wish to 
truncate the name to "N-Trig" or something.  But give them some name. 
Those names simplify driver correlation in user space.

Rafi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/