lkml.org 
[lkml]   [2011]   [Aug]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] input:cyapa: Add new I2C-based input touchpad driver for Cypress I2C touchpad devices
Hi Dudley,

Thanks for the driver, please find some comments on the MT implementation below.

> diff --git a/drivers/input/mouse/cypress_i2c.c b/drivers/input/mouse/cypress_i2c.c
> new file mode 100644
> index 0000000..61d5f7f
> --- /dev/null
> +++ b/drivers/input/mouse/cypress_i2c.c
> @@ -0,0 +1,1721 @@
> +/*
> + * Cypress APA trackpad with I2C interface
> + *
> + * Copyright (C) 2009 Compulab, Ltd.
> + * Dudley Du <dudl@cypress.com>
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License. See the file COPYING in the main directory of this archive for
> + * more details.
> + */
> +
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/i2c.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/input.h>
> +#include <linux/delay.h>
> +#include <linux/workqueue.h>
> +#include <linux/slab.h>
> +#include <linux/gpio.h>
> +#include <linux/spinlock.h>
> +#include <linux/mutex.h>
> +#include <linux/uaccess.h>
> +#include <linux/miscdevice.h>
> +#include <linux/fs.h>
> +#include <linux/input/mt.h>
> +
> +#include <linux/cyapa.h>
> +
> +
> +/* DEBUG: debug switch macro */
> +#define DBG_CYAPA_READ_BLOCK_DATA 0
> +
> +
> +/*
> + * Cypress I2C APA trackpad driver version is defined as below:
> + * CYAPA_MAJOR_VER.CYAPA_MINOR_VER.CYAPA_REVISION_VER
> + */
> +#define CYAPA_MAJOR_VER 1
> +#define CYAPA_MINOR_VER 1
> +#define CYAPA_REVISION_VER 0

Versions are best handled by git.

> +
> +#define CYAPA_MT_MAX_TOUCH 255
> +#define CYAPA_MT_MAX_WIDTH 255
> +
> +#define MAX_FINGERS 5
> +#define CYAPA_TOOL_WIDTH 50
> +#define CYAPA_DEFAULT_TOUCH_PRESSURE 50
> +#define CYAPA_MT_TOUCH_MAJOR 50

Most of the constants above seem unused.

> +/*
> + * In the special case, where a finger is removed and makes contact
> + * between two packets, there will be two touches for that finger,
> + * with different tracking_ids.
> + * Thus, the maximum number of slots must be twice the maximum number
> + * of fingers.
> + */
> +#define MAX_MT_SLOTS (2 * MAX_FINGERS)

Alternatively, it could be equal to the number of tracking ids
recycled by the device, i.e., 16.

> +
> +/* When in IRQ mode read the device every THREAD_IRQ_SLEEP_SECS */
> +#define CYAPA_THREAD_IRQ_SLEEP_SECS 2
> +#define CYAPA_THREAD_IRQ_SLEEP_MSECS (CYAPA_THREAD_IRQ_SLEEP_SECS * MSEC_PER_SEC)
> +/*
> + * When in Polling mode and no data received for CYAPA_NO_DATA_THRES msecs
> + * reduce the polling rate to CYAPA_NO_DATA_SLEEP_MSECS
> + */
> +#define CYAPA_NO_DATA_THRES (MSEC_PER_SEC)
> +#define CYAPA_NO_DATA_SLEEP_MSECS (MSEC_PER_SEC / 4)

As Dmitry already pointed out, polling mode should be skipped altogether.

> + * APA trackpad device states.
> + * Used in register 0x00, bit1-0, DeviceStatus field.
> + */
> +#define CYAPA_MAX_TOUCHES (MAX_FINGERS)

Why two variables with the same meaning?

> +#define CYAPA_ONE_TIME_GESTURES (1)

Gestures should be removed since the are not propagated.

> +
> +struct cyapa_touch {
> + int x;
> + int y;
> + int pressure;
> + int tracking_id;
> +};

There is no need to store the touches separately, cyapa_touch_gen3 will suffice.

> +
> +struct cyapa_gesture {
> + u8 id;
> + u8 param1;
> + u8 param2;
> +};

This one falls out.

> +
> +struct cyapa_touch_gen3 {
> + /*
> + * high bits or x/y position value
> + * bit 7 - 4: high 4 bits of x position value
> + * bit 3 - 0: high 4 bits of y position value
> + */
> + u8 xy;
> + u8 x; /* low 8 bits of x position value. */
> + u8 y; /* low 8 bits of y position value. */
> + u8 pressure;
> + /*
> + * The range of tracking_id is 0 - 15,
> + * it is incremented every time a finger makes contact
> + * with the trackpad.
> + */
> + u8 tracking_id;
> +};

Since the set of tracking ids is so limited, the simplest
implementation is to equate the device tracking ids with the MT
slots. A book-keeping bit field can be used to keep track of what
slots should be cleared in a round.

> +
> +struct cyapa_reg_data_gen3 {
> + /*
> + * bit 0 - 1: device status
> + * bit 3 - 2: power mode
> + * bit 6 - 4: reserved
> + * bit 7: interrupt valid bit
> + */
> + u8 device_status;
> + /*
> + * bit 7 - 4: number of fingers currently touching pad
> + * bit 3: valid data check bit
> + * bit 2: middle mechanism button state if exists
> + * bit 1: right mechanism button state if exists
> + * bit 0: left mechanism button state if exists
> + */
> + u8 finger_btn;
> + struct cyapa_touch_gen3 touches[CYAPA_MAX_TOUCHES];
> +};
> +
> +union cyapa_reg_data {
> + struct cyapa_reg_data_gen3 gen3_data;
> +};

This one goes...

> +
> +struct cyapa_report_data {
> + u8 button;
> + u8 reserved1;
> + u8 reserved2;
> + u8 avg_pressure;
> + int rel_deltaX;
> + int rel_deltaY;
> +
> + int touch_fingers;
> + struct cyapa_touch touches[CYAPA_MAX_TOUCHES];
> +
> + int gesture_count;
> + struct cyapa_gesture gestures[CYAPA_ONE_TIME_GESTURES];
> +};

This struct wont be needed.

> +
> +
> +struct cyapa_mt_slot {
> + struct cyapa_touch contact;
> + bool touch_state; /* true: is touched, false: not touched. */
> + bool slot_updated;
> +};

This struct wont be needed.

> +
> +/* The main device structure */
> +struct cyapa_i2c {
> + /* synchronize i2c bus operations. */
> + struct semaphore reg_io_sem;
> + /* synchronize accessing members of cyapa_i2c data structure. */
> + spinlock_t miscdev_spinlock;
> + /* synchronize accessing and updating file->f_pos. */
> + struct mutex misc_mutex;
> + int misc_open_count;
> + /* indicate interrupt enabled by cyapa driver. */
> + bool irq_enabled;
> + /* indicate interrupt enabled by trackpad device. */
> + bool bl_irq_enable;
> + enum cyapa_work_mode fw_work_mode;
> +
> + struct i2c_client *client;
> + struct input_dev *input;
> + struct delayed_work dwork;
> + struct work_struct detect_work;
> + struct workqueue_struct *detect_wq;
> + enum cyapa_detect_status detect_status;
> + /* synchronize access to dwork. */
> + spinlock_t lock;
> + int no_data_count;
> + int scan_ms;
> + int open_count;
> +
> + int irq;
> + /* driver using polling mode if failed to request irq. */
> + bool polling_mode_enabled;
> + struct cyapa_platform_data *pdata;
> + unsigned short data_base_offset;
> + unsigned short control_base_offset;
> + unsigned short command_base_offset;
> + unsigned short query_base_offset;
> +
> + struct cyapa_mt_slot mt_slots[MAX_MT_SLOTS];

This can be skipped.

> +
> + /* read from query data region. */
> + char product_id[16];
> + unsigned char capability[14];
> + unsigned char fw_maj_ver; /* firmware major version. */
> + unsigned char fw_min_ver; /* firmware minor version. */
> + unsigned char hw_maj_ver; /* hardware major version. */
> + unsigned char hw_min_ver; /* hardware minor version. */
> + int max_abs_x;
> + int max_abs_y;
> + int physical_size_x;
> + int physical_size_y;
> +};
> +
> +#if DBG_CYAPA_READ_BLOCK_DATA
> +#define DUMP_BUF_SIZE (40 * 3 + 20) /* max will dump 40 bytes data. */
> +void cyapa_dump_data_block(const char *func, u8 reg, u8 length, void *data)
> +{
> + char buf[DUMP_BUF_SIZE];
> + unsigned buf_len = sizeof(buf);
> + char *p = buf;
> + int i;
> + int l;
> +
> + l = snprintf(p, buf_len, "reg 0x%04x: ", reg);
> + buf_len -= l;
> + p += l;
> + for (i = 0; i < length && buf_len; i++, p += l, buf_len -= l)
> + l = snprintf(p, buf_len, "%02x ", *((char *)data + i));
> + pr_info("%s: data block length = %d\n", func, length);
> + pr_info("%s: %s\n", func, buf);
> +}
> +
> +void cyapa_dump_report_data(const char *func,
> + struct cyapa_report_data *report_data)
> +{
> + int i;
> +
> + pr_info("%s: ------------------------------------\n", func);
> + pr_info("%s: report_data.button = 0x%02x\n",
> + func, report_data->button);
> + pr_info("%s: report_data.avg_pressure = %d\n",
> + func, report_data->avg_pressure);
> + pr_info("%s: report_data.touch_fingers = %d\n",
> + func, report_data->touch_fingers);
> + for (i = 0; i < report_data->touch_fingers; i++) {
> + pr_info("%s: report_data.touches[%d].x = %d\n",
> + func, i, report_data->touches[i].x);
> + pr_info("%s: report_data.touches[%d].y = %d\n",
> + func, i, report_data->touches[i].y);
> + pr_info("%s: report_data.touches[%d].pressure = %d\n",
> + func, i, report_data->touches[i].pressure);
> + if (report_data->touches[i].tracking_id != -1)
> + pr_info("%s: report_data.touches[%d].tracking_id = %d\n",
> + func, i, report_data->touches[i].tracking_id);
> + }
> + pr_info("%s: report_data.gesture_count = %d\n",
> + func, report_data->gesture_count);
> + for (i = 0; i < report_data->gesture_count; i++) {
> + pr_info("%s: report_data.gestures[%d].id = 0x%02x\n",
> + func, i, report_data->gestures[i].id);
> + pr_info("%s: report_data.gestures[%d].param1 = 0x%02x\n",
> + func, i, report_data->gestures[i].param1);
> + pr_info("%s: report_data.gestures[%d].param2 = 0x%02x\n",
> + func, i, report_data->gestures[i].param2);
> + }
> + pr_info("%s: -------------------------------------\n", func);
> +}
> +#else
> +void cyapa_dump_data_block(const char *func, u8 reg, u8 length, void *data) {}
> +void cyapa_dump_report_data(const char *func,
> + struct cyapa_report_data *report_data) {}
> +#endif

Are these debug functions really necessary?

> +static inline void cyapa_report_fingers(struct input_dev *input, int fingers)
> +{
> + input_report_key(input, BTN_TOOL_FINGER, (fingers == 1));
> + input_report_key(input, BTN_TOOL_DOUBLETAP, (fingers == 2));
> + input_report_key(input, BTN_TOOL_TRIPLETAP, (fingers == 3));
> + input_report_key(input, BTN_TOOL_QUADTAP, (fingers > 3));
> +}

This functionality is found in input_mt_report_finger_count() and
input_mt_report_pointer_emulation().

> +
> +static void cyapa_parse_gen3_data(struct cyapa_i2c *touch,
> + struct cyapa_reg_data_gen3 *reg_data,
> + struct cyapa_report_data *report_data)
> +{
> + int i;
> + int fingers;
> +
> + /* only report left button. */
> + report_data->button = reg_data->finger_btn & OP_DATA_BTN_MASK;
> + report_data->avg_pressure = 0;
> + /* parse number of touching fingers. */
> + fingers = (reg_data->finger_btn >> 4) & 0x0F;
> + report_data->touch_fingers = min(CYAPA_MAX_TOUCHES, fingers);
> +
> + /* parse data for each touched finger. */
> + for (i = 0; i < report_data->touch_fingers; i++) {
> + report_data->touches[i].x =
> + ((reg_data->touches[i].xy & 0xF0) << 4) |
> + reg_data->touches[i].x;
> + report_data->touches[i].y =
> + ((reg_data->touches[i].xy & 0x0F) << 8) |
> + reg_data->touches[i].y;
> + report_data->touches[i].pressure =
> + reg_data->touches[i].pressure;
> + report_data->touches[i].tracking_id =
> + reg_data->touches[i].tracking_id;
> + }
> + report_data->gesture_count = 0;
> +
> + /* DEBUG: dump parsed report data */
> + cyapa_dump_report_data(__func__, report_data);
> +}

This function could be expanded to handle input reporting directly. Something like:

int num_fingers = (reg_data->finger_btn >> 4) & 0x0F;
int mask = 0;

for (i = 0; i < num_fingers; i++) {
const struct cyapa_touch_gen3 *touch = &reg_data->touches[i];
int slot = touch->tracking_id;
int x = ((touch->xy & 0xF0) << 4) | touch->x;
int y = ((touch->xy & 0x0F) << 8) | touch->y;

input_mt_slot(input, slot);
input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
input_report_abs(input, ABS_MT_POSITION_X, x);
input_report_abs(input, ABS_MT_POSITION_Y, y);
input_report_abs(input, ABS_MT_PRESSURE, touch->pressure);

mask |= (1 << slot);
}

for (i = 0; i < MAX_MT_SLOTS; i++) {
if (mask & (1 << i))
continue;
input_mt_slot(input, i);
input_mt_report_slot_state(input, MT_TOOL_FINGER, false);
}


input_mt_report_pointer_emulation(input, true);
input_report_key(input, BTN_LEFT, reg_data->finger_btn & OP_DATA_BTN_MASK);
input_sync(input);

> +
> +
> +static int cyapa_find_mt_slot(struct cyapa_i2c *touch,
> + struct cyapa_touch *contact)
> +{
> + int i;
> + int empty_slot = -1;
> +
> + for (i = 0; i < MAX_MT_SLOTS; i++) {
> + if ((touch->mt_slots[i].contact.tracking_id == contact->tracking_id) &&
> + touch->mt_slots[i].touch_state)
> + return i;
> +
> + if (!touch->mt_slots[i].touch_state && empty_slot == -1)
> + empty_slot = i;
> + }
> +
> + return empty_slot;
> +}

This wont be necessary.

> +
> +static void cyapa_update_mt_slots(struct cyapa_i2c *touch,
> + struct cyapa_report_data *report_data)
> +{
> + int i;
> + int slotnum;
> +
> + for (i = 0; i < report_data->touch_fingers; i++) {
> + slotnum = cyapa_find_mt_slot(touch, &report_data->touches[i]);
> + if (slotnum < 0)
> + continue;
> +
> + memcpy(&touch->mt_slots[slotnum].contact,
> + &report_data->touches[i],
> + sizeof(struct cyapa_touch));
> + touch->mt_slots[slotnum].slot_updated = true;
> + touch->mt_slots[slotnum].touch_state = true;
> + }
> +}

This wont be necessary.

> +
> +static void cyapa_send_mtb_event(struct cyapa_i2c *touch,
> + struct cyapa_report_data *report_data)
> +{
> + int i;
> + struct cyapa_mt_slot *slot;
> + struct input_dev *input = touch->input;
> +
> + cyapa_update_mt_slots(touch, report_data);
> +
> + for (i = 0; i < MAX_MT_SLOTS; i++) {
> + slot = &touch->mt_slots[i];
> + if (!slot->slot_updated)
> + slot->touch_state = false;
> +
> + input_mt_slot(input, i);
> + input_mt_report_slot_state(input, MT_TOOL_FINGER, slot->touch_state);
> + if (slot->touch_state) {
> + input_report_abs(input, ABS_MT_POSITION_X, slot->contact.x);
> + input_report_abs(input, ABS_MT_POSITION_Y, slot->contact.y);
> + input_report_abs(input, ABS_MT_PRESSURE, slot->contact.pressure);
> + }
> + slot->slot_updated = false;
> + }
> +
> + input_mt_report_pointer_emulation(input, true);
> + input_report_key(input, BTN_LEFT, report_data->button);
> + input_sync(input);
> +}

This could be folded into the parsing function.

> +
> +static int cyapa_handle_input_report_data(struct cyapa_i2c *touch,
> + struct cyapa_report_data *report_data)
> +{
> + cyapa_send_mtb_event(touch, report_data);
> +
> + return report_data->touch_fingers | report_data->button;
> +}

This wont be necessary.

> +
> +static bool cyapa_i2c_get_input(struct cyapa_i2c *touch)
> +{
> + int ret_read_size;
> + int read_length;
> + union cyapa_reg_data reg_data;
> + struct cyapa_reg_data_gen3 *gen3_data;
> + struct cyapa_report_data report_data;
> +
> + /* read register data from trackpad. */
> + gen3_data = &reg_data.gen3_data;
> + read_length = (int)sizeof(struct cyapa_reg_data_gen3);
> +
> + ret_read_size = cyapa_i2c_reg_read_block(touch,
> + DATA_REG_START_OFFSET,
> + read_length,
> + (char *)&reg_data);
> + if (ret_read_size < 0)
> + return 0;
> +
> + if (cyapa_verify_data_device(touch, &reg_data) < 0)
> + return 0;
> +
> + /* process and parse raw data read from Trackpad. */
> + cyapa_parse_gen3_data(touch, gen3_data, &report_data);

Reporting could be done here in a straight-forward fashion.

> +
> + /* report data to input subsystem. */
> + return cyapa_handle_input_report_data(touch, &report_data);

Skipping this abstraction entirely.
> +static int cyapa_create_input_dev(struct cyapa_i2c *touch)
> +{
> + int ret;
> + struct input_dev *input = NULL;
> +
> + input = touch->input = input_allocate_device();
> + if (!touch->input) {
> + pr_err("Allocate memory for Input device failed\n");
> + return -ENOMEM;
> + }
> +
> + input->name = "cyapa_i2c_trackpad";
> + input->phys = touch->client->adapter->name;
> + input->id.bustype = BUS_I2C;
> + input->id.version = 1;
> + input->id.product = 0; /* means any product in eventcomm. */
> + input->dev.parent = &touch->client->dev;
> +
> + input->open = cyapa_i2c_open;
> + input->close = cyapa_i2c_close;
> + input_set_drvdata(input, touch);
> +
> + __set_bit(EV_ABS, input->evbit);
> +
> + /*
> + * set and report not-MT axes to support synaptics X Driver.
> + * When multi-fingers on trackpad, only the first finger touch
> + * will be reported as X/Y axes values.
> + */
> + input_set_abs_params(input, ABS_X, 0, touch->max_abs_x, 0, 0);
> + input_set_abs_params(input, ABS_Y, 0, touch->max_abs_y, 0, 0);
> + input_set_abs_params(input, ABS_PRESSURE, 0, 255, 0, 0);
> + input_set_abs_params(input, ABS_TOOL_WIDTH, 0, 255, 0, 0);
> +
> + /* finger position */
> + input_set_abs_params(input, ABS_MT_POSITION_X, 0, touch->max_abs_x, 0, 0);
> + input_set_abs_params(input, ABS_MT_POSITION_Y, 0, touch->max_abs_y, 0, 0);
> + input_set_abs_params(input, ABS_MT_PRESSURE, 0, 255, 0, 0);
> +
> + ret = input_mt_init_slots(input, MAX_MT_SLOTS);
> + if (ret < 0)
> + return ret;
> +
> + if (touch->physical_size_x && touch->physical_size_y) {
> + input_abs_set_res(input, ABS_X,
> + touch->max_abs_x / touch->physical_size_x);
> + input_abs_set_res(input, ABS_Y,
> + touch->max_abs_y / touch->physical_size_y);
> + input_abs_set_res(input, ABS_MT_POSITION_X,
> + touch->max_abs_x / touch->physical_size_x);
> + input_abs_set_res(input, ABS_MT_POSITION_Y,
> + touch->max_abs_y / touch->physical_size_y);
> + }
> +
> + __set_bit(EV_KEY, input->evbit);
> + __set_bit(BTN_TOUCH, input->keybit);
> + __set_bit(BTN_TOOL_FINGER, input->keybit);
> + __set_bit(BTN_TOOL_DOUBLETAP, input->keybit);
> + __set_bit(BTN_TOOL_TRIPLETAP, input->keybit);
> + __set_bit(BTN_TOOL_QUADTAP, input->keybit);
> +
> + __set_bit(BTN_LEFT, input->keybit);
> +
> + /* Register the device in input subsystem */
> + ret = input_register_device(touch->input);
> + if (ret) {
> + pr_err("Input device register failed, %d\n", ret);
> + input_free_device(input);
> + }
> +
> + return ret;
> +}
> +

Thanks,
Henrik


\
 
 \ /
  Last update: 2011-08-25 20:07    [W:0.183 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site