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

[Rafi Rubin <rafi@seas.upenn.edu>]

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