On Tue, 19 May 2009, Stéphane Chatty wrote:
> +/*
> + this driver is aimed at two firmware versions in circulation:
> + - dual pen/finger single touch
> + - finger multitouch, pen not working
> +*/
Please use kernel-style comments (i.e. stars on each line).
> + case 0xff000000:
> + /* we do not want to map these */
Why?
> +/*
> + * this function is called upon all reports
> + * so that we can filter contact point information,
> + * decide whether we are in multi or single touch mode
> + * and call input_mt_sync after each point if necessary
> + */
> +static int ntrig_event (struct hid_device *hid, struct hid_field *field,
> + struct hid_usage *usage, __s32 value)
> +{
> + static __s32 x, y, id, w, h;
> + static char reading_a_point = 0, found_contact_id = 0;
> + struct input_dev *input = field->hidinput->input;
Why are these variables static? This will break horribly if multiple
devices are connected to the system, right?
> + default:
> + hidinput_hid_event(hid, field, usage, value);
Just return 1; here?
> + }
> + }
> + if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)
> + hid->hiddev_hid_event(hid, field, usage, value);
> +
> + return 1;
And return 0; here and let the HID core do all the rest (passing to
hidinput and hiddev, etc) if necessary?
Thanks,
--
Jiri Kosina
SUSE Labs
