Hi Dmitry,
Thanks for your review.
2011/8/1 Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>:
> Hi Zhang,
>
> On Tue, Jul 26, 2011 at 05:25:54PM +0800, Zhang Jiejing wrote:
>> +
>> + if (buf[0] == REPORT_MODE_SINGLE) {
>> + input_report_abs(input_dev, ABS_X, x);
>> + input_report_abs(input_dev, ABS_Y, y);
>> + input_report_key(input_dev, BTN_TOUCH, !!state);
>> + input_sync(input_dev);
>> + return IRQ_HANDLED;
>
> In what cases does the device signal REPORT_MODE_SINGLE? When only one
> finger touches the surface? Because we should still be using MT protocol
> even if at the moment only one finger touches the surface. And it is
> nice to provide non-MT compatibility data for legacy applications
> regardless of the number of contacts currently active (see
> drivers/input/input-mt.c for helpers).
This report single mode was wroted in Chip's I2C programming manual.
but in fact, at least in my touch panel, the event type(buf[0]) was
REPORT_MODE_MTTOUCH regardless one finger or two finger pressed on
panel.
I add this code is in case someone's panel is configured with
REPORT_MODE_SINGLE.
It was no tested, maybe I should remove this part of code ? what's
your opinion ?
>
>> + }
>> +
>> + /* deal with multiple touch */
>> + valid = state & EVENT_VAILD_MASK;
>> + id = (state & EVENT_ID_MASK) >> EVENT_ID_OFFSET;
>> + down = state & EVENT_DOWN_UP;
>> +
>> + if (!valid || id > MAX_SUPPORT_POINTS) {
>> + dev_dbg(&client->dev, "point invalid\n");
>> + return IRQ_HANDLED;
>> + }
>> +
>> + if (down) {
>> + /* since egalax only report one of multi touch event,
>> + * so we need record pervious event with different id
>> + * and report them.*/
>> + events[id].valid = valid;
>> + events[id].status = down;
>> + events[id].x = x;
>> + events[id].y = y;
>> +
>> + for (i = 0; i < MAX_SUPPORT_POINTS; i++) {
>> + if (!events[i].valid)
>> + continue;
>> + dev_dbg(&client->dev, "report id:%d valid:%d x:%d y:%d",
>> + i, valid, x, y);
>> +
>> + input_report_abs(input_dev,
>> + ABS_MT_TRACKING_ID, i);
>> + input_report_abs(input_dev,
>> + ABS_MT_TOUCH_MAJOR, 1);
>
> This does not provide any information, please drop.
will do.
>
>> + input_report_abs(input_dev,
>> + ABS_MT_POSITION_X, events[i].x);
>> + input_report_abs(input_dev,
>> + ABS_MT_POSITION_Y, events[i].y);
>> + input_mt_sync(input_dev);
>
> It would be much better to use MT-B protocol since the device seems to
> provide proper tracking IDs for the contacts.
I will adjust it to MT-B protocol in next version.
>
>> +
>> + __set_bit(EV_ABS, input_dev->evbit);
>> + __set_bit(EV_KEY, input_dev->evbit);
>> + __set_bit(BTN_TOUCH, input_dev->keybit);
>> + __set_bit(ABS_X, input_dev->absbit);
>> + __set_bit(ABS_Y, input_dev->absbit);
>> + input_set_abs_params(input_dev, ABS_X, 0, 32767, 0, 0);
>> + input_set_abs_params(input_dev, ABS_Y, 0, 32767, 0, 0);
>
> You do not set any ABS_MT_* events here so your MT events will be
> suppressed by input core.
ok.
>
>> +
>> +static __devexit int egalax_ts_remove(struct i2c_client *client)
>> +{
>> + struct egalax_ts *data = i2c_get_clientdata(client);
>> +
>> + free_irq(client->irq, data);
>> + input_unregister_device(data->input_dev);
>> + input_free_device(data->input_dev);
>
> No calls to input_free_device() after calling input_unregister_device().
>
ok
> Thanks.
>
> --
> Dmitry
>
Thanks,
Jiejing
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
