2012-06-18 04:13:59

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 00/22 v4] cleanup atmel_mxt_ts

This patchset cleans up the atmel_mxt_ts touchscreen driver.

These patches mostly contain non-controversial changes that were previously
reviewed in previous versions of the patchset. However, based on feedback from
Henrik, the individual patches are now all much smaller and easier to review.

They were tested using an MXT224E, and apply cleanly to input/next.

Whenever these patches (or their descendants) are accepted, there is a larger
patchset full of additional features and more significant changes that will
follow.

Daniel Kurtz (22):
Input: atmel_mxt_ts - set phys to i2c client adapter name
Input: atmel_mxt_ts - use client name for irq
Input: atmel_mxt_ts - detect OOM when creating mt slots
Input: atmel_mxt_ts - warn if sysfs could not be created
Input: atmel_mxt_ts - don't read T5 when dumping objects
Input: atmel_mxt_ts - use scnprintf for object sysfs entry
Input: atmel_mxt_ts - optimize reading objects in object sysfs entry
Input: atmel_mxt_ts - print less overhead when dumping objects
Input: atmel_mxt_ts - print all instances when dumping objects
Input: atmel_mxt_ts - return errors from i2c layer
Input: atmel_mxt_ts - add variable length __mxt_write_reg
Input: atmel_mxt_ts - optimize writing of object table entries
Input: atmel_mxt_ts - read ID information block in one i2c
transaction
Input: atmel_mxt_ts - don't re-read matrix after applying pdata
Input: atmel_mxt_ts - update driver ID info logging
Input: atmel_mxt_ts - add sysfs entries to read fw and hw version
Input: atmel_mxt_ts - simplify event reporting
Input: atmel_mxt_ts - add detail to touchevent debug message
Input: atmel_mxt_ts - cache T9 reportid range when reading object
table
Input: atmel_mxt_ts - use T9 reportid range to init number of mt
slots
Input: atmel_mxt_ts - send all MT-B slots in one input report
Input: atmel_mxt_ts - parse T6 reports

drivers/input/touchscreen/atmel_mxt_ts.c | 449 ++++++++++++++----------------
1 files changed, 212 insertions(+), 237 deletions(-)

--
1.7.7.3


2012-06-18 04:08:50

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 03/22 v4] Input: atmel_mxt_ts - detect OOM when creating mt slots

Hopefully this new code path will never be used, but better safe than
sorry...

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index cb191fe..00b37b1 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1132,7 +1132,9 @@ static int __devinit mxt_probe(struct i2c_client *client,
0, 255, 0, 0);

/* For multi touch */
- input_mt_init_slots(input_dev, MXT_MAX_FINGER);
+ error = input_mt_init_slots(input_dev, MXT_MAX_FINGER);
+ if (error)
+ goto err_free_mem;
input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
0, MXT_MAX_AREA, 0, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
--
1.7.7.3

2012-06-18 04:08:53

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 05/22 v4] Input: atmel_mxt_ts - don't read T5 when dumping objects

T5 is the message processor object. Reading it will only have two
outcomes, neither of which is particularly useful:
1) the message count decrements, and a valid message will be lost
2) an invalid message will be read (reportid == 0xff)

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 0c49a1e..5eee68d 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -262,7 +262,6 @@ struct mxt_data {
static bool mxt_object_readable(unsigned int type)
{
switch (type) {
- case MXT_GEN_MESSAGE_T5:
case MXT_GEN_COMMAND_T6:
case MXT_GEN_POWER_T7:
case MXT_GEN_ACQUIRE_T8:
--
1.7.7.3

2012-06-18 04:09:19

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 07/22 v4] Input: atmel_mxt_ts - optimize reading objects in object sysfs entry

Read each object in a single i2c transaction instead of byte-by-byte

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 35 ++++++++++++-----------------
1 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 472ab29..8c99d6d 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -478,20 +478,6 @@ static int mxt_read_message(struct mxt_data *data,
sizeof(struct mxt_message), message);
}

-static int mxt_read_object(struct mxt_data *data,
- u8 type, u8 offset, u8 *val)
-{
- struct mxt_object *object;
- u16 reg;
-
- object = mxt_get_object(data, type);
- if (!object)
- return -EINVAL;
-
- reg = object->start_address;
- return __mxt_read_reg(data->client, reg + offset, 1, val);
-}
-
static int mxt_write_object(struct mxt_data *data,
u8 type, u8 offset, u8 val)
{
@@ -899,7 +885,14 @@ static ssize_t mxt_object_show(struct device *dev,
int i, j;
int error;
u8 val;
+ u8 *obuf;
+
+ /* Pre-allocate buffer large enough to hold max sized object. */
+ obuf = kmalloc(256, GFP_KERNEL);
+ if (!obuf)
+ return -ENOMEM;

+ error = 0;
for (i = 0; i < data->info.object_num; i++) {
object = data->object_table + i;

@@ -913,20 +906,22 @@ static ssize_t mxt_object_show(struct device *dev,
continue;
}

+ error = __mxt_read_reg(data->client, object->start_address,
+ object->size + 1, obuf);
+ if (error)
+ break;
+
for (j = 0; j < object->size + 1; j++) {
- error = mxt_read_object(data,
- object->type, j, &val);
- if (error)
- return error;
+ val = obuf[j];

count += scnprintf(buf + count, PAGE_SIZE - count,
"\t[%2d]: %02x (%d)\n", j, val, val);
}
-
count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
}

- return count;
+ kfree(obuf);
+ return error ?: count;
}

static int mxt_load_fw(struct device *dev, const char *fn)
--
1.7.7.3

2012-06-18 04:09:32

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 11/22 v4] Input: atmel_mxt_ts - add variable length __mxt_write_reg

The i2c bus requires 4 bytes to do a 1-byte write
(1 byte i2c address + 2 byte offset + 1 byte data).

By taking a length with writes, the driver can amortize transaction
overhead by performing larger transactions where appropriate.

This patch just sets up the new API. Later patches refactor writes
to take advantage of the larger transactions.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 22 +++++++++++++++++-----
1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 1ef3aef..2e49488 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -430,17 +430,24 @@ static int mxt_read_reg(struct i2c_client *client, u16 reg, u8 *val)
return __mxt_read_reg(client, reg, 1, val);
}

-static int mxt_write_reg(struct i2c_client *client, u16 reg, u8 val)
+static int __mxt_write_reg(struct i2c_client *client, u16 reg, u16 len,
+ const void *val)
{
- u8 buf[3];
+ u8 *buf;
+ size_t count;
int ret;

+ count = len + 2;
+ buf = kmalloc(count, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
buf[0] = reg & 0xff;
buf[1] = (reg >> 8) & 0xff;
- buf[2] = val;
+ memcpy(&buf[2], val, len);

- ret = i2c_master_send(client, buf, 3);
- if (ret == 3) {
+ ret = i2c_master_send(client, buf, count);
+ if (ret == count) {
ret = 0;
} else {
if (ret >= 0)
@@ -452,6 +459,11 @@ static int mxt_write_reg(struct i2c_client *client, u16 reg, u8 val)
return ret;
}

+static int mxt_write_reg(struct i2c_client *client, u16 reg, u8 val)
+{
+ return __mxt_write_reg(client, reg, 1, &val);
+}
+
static int mxt_read_object_table(struct i2c_client *client,
u16 reg, u8 *object_buf)
{
--
1.7.7.3

2012-06-18 04:09:29

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 01/22 v4] Input: atmel_mxt_ts - set phys to i2c client adapter name

This allows userspace to more easily distinguish which bus a particular
atmel_mxt_ts device is attached to.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 42e6450..eda9eed 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1106,6 +1106,7 @@ static int __devinit mxt_probe(struct i2c_client *client,
}

input_dev->name = "Atmel maXTouch Touchscreen";
+ input_dev->phys = client->adapter->name;
input_dev->id.bustype = BUS_I2C;
input_dev->dev.parent = &client->dev;
input_dev->open = mxt_input_open;
--
1.7.7.3

2012-06-18 04:09:26

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 09/22 v4] Input: atmel_mxt_ts - print all instances when dumping objects

For objects with multiple instances, dump them all, prepending each with
its "Instance #".

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 30 +++++++++++++++++++++---------
1 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 65fd89b..281fcbd 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -882,7 +882,7 @@ static ssize_t mxt_object_show(struct device *dev,
struct mxt_data *data = dev_get_drvdata(dev);
struct mxt_object *object;
int count = 0;
- int i, j;
+ int i, j, k;
int error;
u8 val;
u8 *obuf;
@@ -902,18 +902,30 @@ static ssize_t mxt_object_show(struct device *dev,
count += scnprintf(buf + count, PAGE_SIZE - count,
"T%u:\n", object->type);

- error = __mxt_read_reg(data->client, object->start_address,
- object->size + 1, obuf);
- if (error)
- break;
+ for (j = 0; j < object->instances + 1; j++) {
+ u16 size = object->size + 1;
+ u16 addr = object->start_address + j * size;
+
+ error = __mxt_read_reg(data->client, addr, size, obuf);
+ if (error)
+ break;

- for (j = 0; j < object->size + 1; j++) {
- val = obuf[j];
+ if (object->instances > 0)
+ count += scnprintf(buf + count,
+ PAGE_SIZE - count,
+ "Instance %u\n", j);

+ for (k = 0; k < object->size + 1; k++) {
+ val = obuf[k];
+
+ count += scnprintf(buf + count,
+ PAGE_SIZE - count,
+ "\t[%2u]: %02x (%d)\n",
+ k, val, val);
+ }
count += scnprintf(buf + count, PAGE_SIZE - count,
- "\t[%2d]: %02x (%d)\n", j, val, val);
+ "\n");
}
- count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
}

kfree(obuf);
--
1.7.7.3

2012-06-18 04:09:23

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 21/22 v4] Input: atmel_mxt_ts - send all MT-B slots in one input report

Each interrupt contains information for all contacts with changing
properties. Process all of this information at once, and send it all in a
a single input report (ie input events ending in EV_SYN/SYN_REPORT).

This patch was tested using an MXT224E.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 460c10f..06f2b7a 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -540,9 +540,6 @@ static void mxt_input_touchevent(struct mxt_data *data,
input_report_abs(input_dev, ABS_MT_PRESSURE, pressure);
input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, area);
}
-
- input_mt_report_pointer_emulation(input_dev, false);
- input_sync(input_dev);
}

static irqreturn_t mxt_interrupt(int irq, void *dev_id)
@@ -554,6 +551,7 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)
u8 reportid;
u8 max_reportid;
u8 min_reportid;
+ bool update_input = false;

do {
if (mxt_read_message(data, &message)) {
@@ -567,12 +565,19 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)
min_reportid = data->T9_reportid_min;
id = reportid - min_reportid;

- if (reportid >= min_reportid && reportid <= max_reportid)
+ if (reportid >= min_reportid && reportid <= max_reportid) {
mxt_input_touchevent(data, &message, id);
- else
+ update_input = true;
+ } else {
mxt_dump_message(dev, &message);
+ }
} while (reportid != 0xff);

+ if (update_input) {
+ input_mt_report_pointer_emulation(data->input_dev, false);
+ input_sync(data->input_dev);
+ }
+
end:
return IRQ_HANDLED;
}
--
1.7.7.3

2012-06-18 04:09:18

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 12/22 v4] Input: atmel_mxt_ts - optimize writing of object table entries

Write each object using a single bulk i2c write transfer.

Signed-off-by: Daniel Kurtz <[email protected]>
Reviewed-by: Joonyoung Shim <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 2e49488..efce50f 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -653,7 +653,8 @@ static int mxt_check_reg_init(struct mxt_data *data)
struct mxt_object *object;
struct device *dev = &data->client->dev;
int index = 0;
- int i, j, config_offset;
+ int i, size;
+ int ret;

if (!pdata->config) {
dev_dbg(dev, "No cfg data defined, skipping reg init\n");
@@ -666,18 +667,17 @@ static int mxt_check_reg_init(struct mxt_data *data)
if (!mxt_object_writable(object->type))
continue;

- for (j = 0;
- j < (object->size + 1) * (object->instances + 1);
- j++) {
- config_offset = index + j;
- if (config_offset > pdata->config_length) {
- dev_err(dev, "Not enough config data!\n");
- return -EINVAL;
- }
- mxt_write_object(data, object->type, j,
- pdata->config[config_offset]);
+ size = (object->size + 1) * (object->instances + 1);
+ if (index + size > pdata->config_length) {
+ dev_err(dev, "Not enough config data!\n");
+ return -EINVAL;
}
- index += (object->size + 1) * (object->instances + 1);
+
+ ret = __mxt_write_reg(data->client, object->start_address,
+ size, &pdata->config[index]);
+ if (ret)
+ return ret;
+ index += size;
}

return 0;
--
1.7.7.3

2012-06-18 04:09:15

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 18/22 v4] Input: atmel_mxt_ts - add detail to touchevent debug message

Update the debug message:
* print inidividual status bits
* print the pressure value
* use '%u' for unsigned quantities

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 17 ++++++++++++++---
1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 02ff914..d71ecf0 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -195,6 +195,7 @@
#define MXT_BOOT_STATUS_MASK 0x3f

/* Touch status */
+#define MXT_UNGRIP (1 << 0)
#define MXT_SUPPRESS (1 << 1)
#define MXT_AMP (1 << 2)
#define MXT_VECTOR (1 << 3)
@@ -524,9 +525,19 @@ static void mxt_input_touchevent(struct mxt_data *data,
area = message->message[4];
pressure = message->message[5];

- dev_dbg(dev, "[%d] %s x: %d, y: %d, area: %d\n", id,
- status & MXT_MOVE ? "moved" : "pressed",
- x, y, area);
+ dev_dbg(dev,
+ "[%u] %c%c%c%c%c%c%c%c x: %5u y: %5u area: %3u amp: %3u\n",
+ id,
+ (status & MXT_DETECT) ? 'D' : '.',
+ (status & MXT_PRESS) ? 'P' : '.',
+ (status & MXT_RELEASE) ? 'R' : '.',
+ (status & MXT_MOVE) ? 'M' : '.',
+ (status & MXT_VECTOR) ? 'V' : '.',
+ (status & MXT_AMP) ? 'A' : '.',
+ (status & MXT_SUPPRESS) ? 'S' : '.',
+ (status & MXT_UNGRIP) ? 'U' : '.',
+ x, y, area, pressure);
+
input_mt_slot(input_dev, id);
input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
status & MXT_DETECT);
--
1.7.7.3

2012-06-18 04:11:09

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 22/22 v4] Input: atmel_mxt_ts - parse T6 reports

The normal messages sent after boot or NVRAM update are T6 reports,
containing a status, and the config memory checksum. Parse them and dump
a useful info message.

This patch tested on an MXT224E.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 21 ++++++++++++++-------
1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 06f2b7a..7b27e85 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -247,6 +247,7 @@ struct mxt_data {
unsigned int max_y;

/* Cached parameters from object table */
+ u8 T6_reportid;
u8 T9_reportid_min;
u8 T9_reportid_max;
};
@@ -549,8 +550,6 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)
struct device *dev = &data->client->dev;
int id;
u8 reportid;
- u8 max_reportid;
- u8 min_reportid;
bool update_input = false;

do {
@@ -561,11 +560,15 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)

reportid = message.reportid;

- max_reportid = data->T9_reportid_max;
- min_reportid = data->T9_reportid_min;
- id = reportid - min_reportid;
-
- if (reportid >= min_reportid && reportid <= max_reportid) {
+ if (reportid == data->T6_reportid) {
+ unsigned csum = message.message[1] |
+ (message.message[2] << 8) |
+ (message.message[3] << 16);
+ dev_info(dev, "Status: %02x Config Checksum: %06x\n",
+ message.message[0], csum);
+ } else if (reportid >= data->T9_reportid_min &&
+ reportid <= data->T9_reportid_max) {
+ id = reportid - data->T9_reportid_min;
mxt_input_touchevent(data, &message, id);
update_input = true;
} else {
@@ -743,6 +746,9 @@ static int mxt_get_object_table(struct mxt_data *data)
object->instances + 1, min_id, max_id);

switch (object->type) {
+ case MXT_GEN_COMMAND_T6:
+ data->T6_reportid = min_id;
+ break;
case MXT_TOUCH_MULTI_T9:
data->T9_reportid_min = min_id;
data->T9_reportid_max = max_id;
@@ -990,6 +996,7 @@ static ssize_t mxt_update_fw_store(struct device *dev,
/* Destroy old object table and any cached fields */
kfree(data->object_table);
data->object_table = NULL;
+ data->T6_reportid = 0;
data->T9_reportid_min = 0;
data->T9_reportid_max = 0;

--
1.7.7.3

2012-06-18 04:11:06

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 17/22 v4] Input: atmel_mxt_ts - simplify event reporting

Instead of carrying around per-finger state in the driver instance, just
report each finger as it arrives to the input layer, and let the input
layer (evdev) hold the event state (which it does anyway).

Note: this driver does not really do MT-B properly. Each input report
(a group of input events followed by a SYN_REPORT) only contains data for
a single contact. When multiple fingers are present on a device, each is
properly reported in its own MT_SLOT. However, there is only ever one
MT_SLOT per SYN_REPORT. This is fixed in a subsequent patch.

This patch was tested with an mXT224E.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 89 ++++-------------------------
1 files changed, 13 insertions(+), 76 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 0d769de..02ff914 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -239,14 +239,6 @@ struct mxt_message {
u8 message[7];
};

-struct mxt_finger {
- int status;
- int x;
- int y;
- int area;
- int pressure;
-};
-
/* Each client has this additional data */
struct mxt_data {
struct i2c_client *client;
@@ -254,7 +246,6 @@ struct mxt_data {
const struct mxt_platform_data *pdata;
struct mxt_object *object_table;
struct mxt_info info;
- struct mxt_finger finger[MXT_MAX_FINGER];
unsigned int irq;
unsigned int max_x;
unsigned int max_y;
@@ -512,75 +503,17 @@ static int mxt_write_object(struct mxt_data *data,
return mxt_write_reg(data->client, reg + offset, val);
}

-static void mxt_input_report(struct mxt_data *data, int single_id)
-{
- struct mxt_finger *finger = data->finger;
- struct input_dev *input_dev = data->input_dev;
- int status = finger[single_id].status;
- int finger_num = 0;
- int id;
-
- for (id = 0; id < MXT_MAX_FINGER; id++) {
- if (!finger[id].status)
- continue;
-
- input_mt_slot(input_dev, id);
- input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
- finger[id].status != MXT_RELEASE);
-
- if (finger[id].status != MXT_RELEASE) {
- finger_num++;
- input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
- finger[id].area);
- input_report_abs(input_dev, ABS_MT_POSITION_X,
- finger[id].x);
- input_report_abs(input_dev, ABS_MT_POSITION_Y,
- finger[id].y);
- input_report_abs(input_dev, ABS_MT_PRESSURE,
- finger[id].pressure);
- } else {
- finger[id].status = 0;
- }
- }
-
- input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
-
- if (status != MXT_RELEASE) {
- input_report_abs(input_dev, ABS_X, finger[single_id].x);
- input_report_abs(input_dev, ABS_Y, finger[single_id].y);
- input_report_abs(input_dev,
- ABS_PRESSURE, finger[single_id].pressure);
- }
-
- input_sync(input_dev);
-}
-
static void mxt_input_touchevent(struct mxt_data *data,
struct mxt_message *message, int id)
{
- struct mxt_finger *finger = data->finger;
struct device *dev = &data->client->dev;
u8 status = message->message[0];
+ struct input_dev *input_dev = data->input_dev;
int x;
int y;
int area;
int pressure;

- /* Check the touch is present on the screen */
- if (!(status & MXT_DETECT)) {
- if (status & MXT_RELEASE) {
- dev_dbg(dev, "[%d] released\n", id);
-
- finger[id].status = MXT_RELEASE;
- mxt_input_report(data, id);
- }
- return;
- }
-
- /* Check only AMP detection */
- if (!(status & (MXT_PRESS | MXT_MOVE)))
- return;
-
x = (message->message[1] << 4) | ((message->message[3] >> 4) & 0xf);
y = (message->message[2] << 4) | ((message->message[3] & 0xf));
if (data->max_x < 1024)
@@ -594,15 +527,19 @@ static void mxt_input_touchevent(struct mxt_data *data,
dev_dbg(dev, "[%d] %s x: %d, y: %d, area: %d\n", id,
status & MXT_MOVE ? "moved" : "pressed",
x, y, area);
+ input_mt_slot(input_dev, id);
+ input_mt_report_slot_state(input_dev, MT_TOOL_FINGER,
+ status & MXT_DETECT);
+
+ if (status & MXT_DETECT) {
+ input_report_abs(input_dev, ABS_MT_POSITION_X, x);
+ input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
+ input_report_abs(input_dev, ABS_MT_PRESSURE, pressure);
+ input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, area);
+ }

- finger[id].status = status & MXT_MOVE ?
- MXT_MOVE : MXT_PRESS;
- finger[id].x = x;
- finger[id].y = y;
- finger[id].area = area;
- finger[id].pressure = pressure;
-
- mxt_input_report(data, id);
+ input_mt_report_pointer_emulation(input_dev, false);
+ input_sync(input_dev);
}

static irqreturn_t mxt_interrupt(int irq, void *dev_id)
--
1.7.7.3

2012-06-18 04:09:14

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 14/22 v4] Input: atmel_mxt_ts - don't re-read matrix after applying pdata

The matrix x/y size in the Info ID Block represents the number of x/y
trace lines on the device. There is no need to re-read them after
applying pdata config, since pdata only configures the object table
etnries. The matrix size read from the ID can only be updated by a
firmware update.

As a side affect, to squash a compiler warning, remove the 1-byte limited
mxt_read_reg() since it no longer has any callers.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 17 -----------------
1 files changed, 0 insertions(+), 17 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 87004ec..214bf8d 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -426,11 +426,6 @@ static int __mxt_read_reg(struct i2c_client *client,
return ret;
}

-static int mxt_read_reg(struct i2c_client *client, u16 reg, u8 *val)
-{
- return __mxt_read_reg(client, reg, 1, val);
-}
-
static int __mxt_write_reg(struct i2c_client *client, u16 reg, u16 len,
const void *val)
{
@@ -805,7 +800,6 @@ static int mxt_initialize(struct mxt_data *data)
struct i2c_client *client = data->client;
struct mxt_info *info = &data->info;
int error;
- u8 val;

error = mxt_get_info(data);
if (error)
@@ -842,17 +836,6 @@ static int mxt_initialize(struct mxt_data *data)
MXT_COMMAND_RESET, 1);
msleep(MXT_RESET_TIME);

- /* Update matrix size at info struct */
- error = mxt_read_reg(client, MXT_MATRIX_X_SIZE, &val);
- if (error)
- return error;
- info->matrix_xsize = val;
-
- error = mxt_read_reg(client, MXT_MATRIX_Y_SIZE, &val);
- if (error)
- return error;
- info->matrix_ysize = val;
-
dev_info(&client->dev,
"Family ID: %d Variant ID: %d Version: %d Build: %d\n",
info->family_id, info->variant_id, info->version,
--
1.7.7.3

2012-06-18 04:09:12

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 19/22 v4] Input: atmel_mxt_ts - cache T9 reportid range when reading object table

Streamline interrupt processing by caching the T9 reportid range when
first reading the object table.

In the process, refactor reading the object descriptor table.
First, since the object_table entries are now exactly the same layout
in device memory and in the driver, allocate an appropriately sized
array and fetch the entire table directly into it in a single i2c
transaction. Since a 6 byte table object requires 10 bytes to read,
doing this dramatically reduces overhead.

Note: The cached T9 reportid's are initialized to 0, which is an invalid
reportid. Thus, the checks in the interrupt handler will always fail for
devices that do not support the T9 object. Therefore, after doing a
firmware update, the old object table is destroyed and all cached object
values are reset to 0, before reading the new object table, in case
the new firmware does not have the old objects.

This patch tested on an MXT224E.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 80 +++++++++++++++++-------------
1 files changed, 45 insertions(+), 35 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index d71ecf0..f5ca237 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -227,13 +227,10 @@ struct mxt_info {
struct mxt_object {
u8 type;
u16 start_address;
- u8 size;
- u8 instances;
+ u8 size; /* Size of each instance - 1 */
+ u8 instances; /* Number of instances - 1 */
u8 num_report_ids;
-
- /* to map object and message */
- u8 max_reportid;
-};
+} __packed;

struct mxt_message {
u8 reportid;
@@ -250,6 +247,10 @@ struct mxt_data {
unsigned int irq;
unsigned int max_x;
unsigned int max_y;
+
+ /* Cached parameters from object table */
+ u8 T9_reportid_min;
+ u8 T9_reportid_max;
};

static bool mxt_object_readable(unsigned int type)
@@ -452,13 +453,6 @@ static int mxt_write_reg(struct i2c_client *client, u16 reg, u8 val)
return __mxt_write_reg(client, reg, 1, &val);
}

-static int mxt_read_object_table(struct i2c_client *client,
- u16 reg, u8 *object_buf)
-{
- return __mxt_read_reg(client, reg, MXT_OBJECT_SIZE,
- object_buf);
-}
-
static struct mxt_object *
mxt_get_object(struct mxt_data *data, u8 type)
{
@@ -557,7 +551,6 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)
{
struct mxt_data *data = dev_id;
struct mxt_message message;
- struct mxt_object *object;
struct device *dev = &data->client->dev;
int id;
u8 reportid;
@@ -572,13 +565,8 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)

reportid = message.reportid;

- /* whether reportid is thing of MXT_TOUCH_MULTI_T9 */
- object = mxt_get_object(data, MXT_TOUCH_MULTI_T9);
- if (!object)
- goto end;
-
- max_reportid = object->max_reportid;
- min_reportid = max_reportid - object->num_report_ids + 1;
+ max_reportid = data->T9_reportid_max;
+ min_reportid = data->T9_reportid_min;
id = reportid - min_reportid;

if (reportid >= min_reportid && reportid <= max_reportid)
@@ -713,30 +701,49 @@ static int mxt_get_info(struct mxt_data *data)

static int mxt_get_object_table(struct mxt_data *data)
{
+ struct i2c_client *client = data->client;
+ size_t table_size;
int error;
int i;
- u16 reg;
- u8 reportid = 0;
- u8 buf[MXT_OBJECT_SIZE];
+ u8 reportid;

+ table_size = data->info.object_num * sizeof(struct mxt_object);
+ error = __mxt_read_reg(client, MXT_OBJECT_START, table_size,
+ data->object_table);
+ if (error) {
+ kfree(data->object_table);
+ data->object_table = NULL;
+ return error;
+ }
+
+ /* Valid Report IDs start counting from 1 */
+ reportid = 1;
for (i = 0; i < data->info.object_num; i++) {
struct mxt_object *object = data->object_table + i;
+ u8 min_id, max_id;

- reg = MXT_OBJECT_START + MXT_OBJECT_SIZE * i;
- error = mxt_read_object_table(data->client, reg, buf);
- if (error)
- return error;
-
- object->type = buf[0];
- object->start_address = (buf[2] << 8) | buf[1];
- object->size = buf[3];
- object->instances = buf[4];
- object->num_report_ids = buf[5];
+ le16_to_cpus(&object->start_address);

if (object->num_report_ids) {
+ min_id = reportid;
reportid += object->num_report_ids *
(object->instances + 1);
- object->max_reportid = reportid;
+ max_id = reportid - 1;
+ } else {
+ min_id = 0;
+ max_id = 0;
+ }
+
+ dev_info(&data->client->dev,
+ "Type %2d Start %3d Size %3d Instances %2d ReportIDs %3u : %3u\n",
+ object->type, object->start_address, object->size + 1,
+ object->instances + 1, min_id, max_id);
+
+ switch (object->type) {
+ case MXT_TOUCH_MULTI_T9:
+ data->T9_reportid_min = min_id;
+ data->T9_reportid_max = max_id;
+ break;
}
}

@@ -977,8 +984,11 @@ static ssize_t mxt_update_fw_store(struct device *dev,
/* Wait for reset */
msleep(MXT_FWRESET_TIME);

+ /* Destroy old object table and any cached fields */
kfree(data->object_table);
data->object_table = NULL;
+ data->T9_reportid_min = 0;
+ data->T9_reportid_max = 0;

mxt_initialize(data);
}
--
1.7.7.3

2012-06-18 04:12:20

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 13/22 v4] Input: atmel_mxt_ts - read ID information block in one i2c transaction

Reading the whole info block in one i2c transaction speeds up driver
probe significantly, especially on slower i2c busses.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 26 +++-----------------------
1 files changed, 3 insertions(+), 23 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index efce50f..87004ec 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -36,6 +36,7 @@
#define MXT_FW_NAME "maxtouch.fw"

/* Registers */
+#define MXT_INFO 0x00
#define MXT_FAMILY_ID 0x00
#define MXT_VARIANT_ID 0x01
#define MXT_VERSION 0x02
@@ -758,32 +759,11 @@ static int mxt_get_info(struct mxt_data *data)
struct i2c_client *client = data->client;
struct mxt_info *info = &data->info;
int error;
- u8 val;
-
- error = mxt_read_reg(client, MXT_FAMILY_ID, &val);
- if (error)
- return error;
- info->family_id = val;
-
- error = mxt_read_reg(client, MXT_VARIANT_ID, &val);
- if (error)
- return error;
- info->variant_id = val;
-
- error = mxt_read_reg(client, MXT_VERSION, &val);
- if (error)
- return error;
- info->version = val;
-
- error = mxt_read_reg(client, MXT_BUILD, &val);
- if (error)
- return error;
- info->build = val;

- error = mxt_read_reg(client, MXT_OBJECT_NUM, &val);
+ /* Read 7-byte info block starting at address 0 */
+ error = __mxt_read_reg(client, MXT_INFO, sizeof(*info), info);
if (error)
return error;
- info->object_num = val;

return 0;
}
--
1.7.7.3

2012-06-18 04:12:17

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 16/22 v4] Input: atmel_mxt_ts - add sysfs entries to read fw and hw version

Make firmware and hardware version strings available to userspace.
This is useful, for example, to allow a userspace program to implement
a firwmare update policy.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 9200cf4..0d769de 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -863,6 +863,26 @@ static void mxt_calc_resolution(struct mxt_data *data)
}
}

+/* Firmware Version is returned as Major.Minor.Build */
+static ssize_t mxt_fw_version_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct mxt_data *data = dev_get_drvdata(dev);
+ struct mxt_info *info = &data->info;
+ return scnprintf(buf, PAGE_SIZE, "%u.%u.%02X\n",
+ info->version >> 4, info->version & 0xf, info->build);
+}
+
+/* Hardware Version is returned as FamilyID.VariantID */
+static ssize_t mxt_hw_version_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct mxt_data *data = dev_get_drvdata(dev);
+ struct mxt_info *info = &data->info;
+ return scnprintf(buf, PAGE_SIZE, "%u.%u\n",
+ info->family_id, info->variant_id);
+}
+
static ssize_t mxt_object_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -1024,10 +1044,14 @@ static ssize_t mxt_update_fw_store(struct device *dev,
return count;
}

+static DEVICE_ATTR(fw_version, S_IRUGO, mxt_fw_version_show, NULL);
+static DEVICE_ATTR(hw_version, S_IRUGO, mxt_hw_version_show, NULL);
static DEVICE_ATTR(object, S_IRUGO, mxt_object_show, NULL);
static DEVICE_ATTR(update_fw, S_IWUSR, NULL, mxt_update_fw_store);

static struct attribute *mxt_attrs[] = {
+ &dev_attr_fw_version.attr,
+ &dev_attr_hw_version.attr,
&dev_attr_object.attr,
&dev_attr_update_fw.attr,
NULL
--
1.7.7.3

2012-06-18 04:12:16

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 08/22 v4] Input: atmel_mxt_ts - print less overhead when dumping objects

Conserve limited (PAGE_SIZE) sysfs output buffer space by only showing
readable objects and not printing the object's index, which is not useful
to userspace.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 8c99d6d..65fd89b 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -896,15 +896,11 @@ static ssize_t mxt_object_show(struct device *dev,
for (i = 0; i < data->info.object_num; i++) {
object = data->object_table + i;

- count += scnprintf(buf + count, PAGE_SIZE - count,
- "Object[%d] (Type %d)\n",
- i + 1, object->type);
-
- if (!mxt_object_readable(object->type)) {
- count += scnprintf(buf + count, PAGE_SIZE - count,
- "\n");
+ if (!mxt_object_readable(object->type))
continue;
- }
+
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "T%u:\n", object->type);

error = __mxt_read_reg(data->client, object->start_address,
object->size + 1, obuf);
--
1.7.7.3

2012-06-18 04:12:13

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 10/22 v4] Input: atmel_mxt_ts - return errors from i2c layer

The i2c layer can report a variety of errors, including -ENXIO for an i2c
NAK. Instead of treating them all as -EIO, pass the actual i2c layer
error up to the caller.

However, still report as -EIO the unlikely case that a transaction was
partially completed, and no error message was returned from i2c_*().

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 28 ++++++++++++++++++++--------
1 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 281fcbd..1ef3aef 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -395,6 +395,7 @@ static int __mxt_read_reg(struct i2c_client *client,
{
struct i2c_msg xfer[2];
u8 buf[2];
+ int ret;

buf[0] = reg & 0xff;
buf[1] = (reg >> 8) & 0xff;
@@ -411,12 +412,17 @@ static int __mxt_read_reg(struct i2c_client *client,
xfer[1].len = len;
xfer[1].buf = val;

- if (i2c_transfer(client->adapter, xfer, 2) != 2) {
- dev_err(&client->dev, "%s: i2c transfer failed\n", __func__);
- return -EIO;
+ ret = i2c_transfer(client->adapter, xfer, 2);
+ if (ret == 2) {
+ ret = 0;
+ } else {
+ if (ret >= 0)
+ ret = -EIO;
+ dev_err(&client->dev, "%s: i2c transfer failed (%d)\n",
+ __func__, ret);
}

- return 0;
+ return ret;
}

static int mxt_read_reg(struct i2c_client *client, u16 reg, u8 *val)
@@ -427,17 +433,23 @@ static int mxt_read_reg(struct i2c_client *client, u16 reg, u8 *val)
static int mxt_write_reg(struct i2c_client *client, u16 reg, u8 val)
{
u8 buf[3];
+ int ret;

buf[0] = reg & 0xff;
buf[1] = (reg >> 8) & 0xff;
buf[2] = val;

- if (i2c_master_send(client, buf, 3) != 3) {
- dev_err(&client->dev, "%s: i2c send failed\n", __func__);
- return -EIO;
+ ret = i2c_master_send(client, buf, 3);
+ if (ret == 3) {
+ ret = 0;
+ } else {
+ if (ret >= 0)
+ ret = -EIO;
+ dev_err(&client->dev, "%s: i2c send failed (%d)\n",
+ __func__, ret);
}

- return 0;
+ return ret;
}

static int mxt_read_object_table(struct i2c_client *client,
--
1.7.7.3

2012-06-18 04:13:32

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 06/22 v4] Input: atmel_mxt_ts - use scnprintf for object sysfs entry

Using scnprintf() is a cleaner way to ensure that we don't overwrite the
PAGE_SIZE sysfs output buffer.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 16 ++++------------
1 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 5eee68d..472ab29 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -903,17 +903,13 @@ static ssize_t mxt_object_show(struct device *dev,
for (i = 0; i < data->info.object_num; i++) {
object = data->object_table + i;

- count += snprintf(buf + count, PAGE_SIZE - count,
+ count += scnprintf(buf + count, PAGE_SIZE - count,
"Object[%d] (Type %d)\n",
i + 1, object->type);
- if (count >= PAGE_SIZE)
- return PAGE_SIZE - 1;

if (!mxt_object_readable(object->type)) {
- count += snprintf(buf + count, PAGE_SIZE - count,
+ count += scnprintf(buf + count, PAGE_SIZE - count,
"\n");
- if (count >= PAGE_SIZE)
- return PAGE_SIZE - 1;
continue;
}

@@ -923,15 +919,11 @@ static ssize_t mxt_object_show(struct device *dev,
if (error)
return error;

- count += snprintf(buf + count, PAGE_SIZE - count,
+ count += scnprintf(buf + count, PAGE_SIZE - count,
"\t[%2d]: %02x (%d)\n", j, val, val);
- if (count >= PAGE_SIZE)
- return PAGE_SIZE - 1;
}

- count += snprintf(buf + count, PAGE_SIZE - count, "\n");
- if (count >= PAGE_SIZE)
- return PAGE_SIZE - 1;
+ count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
}

return count;
--
1.7.7.3

2012-06-18 04:13:30

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 15/22 v4] Input: atmel_mxt_ts - update driver ID info logging

Print unsigned values as '%u'.
Also, parse and print the firmware version in its canonical format, as
suggested by Nick Dyer.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 214bf8d..9200cf4 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -837,12 +837,12 @@ static int mxt_initialize(struct mxt_data *data)
msleep(MXT_RESET_TIME);

dev_info(&client->dev,
- "Family ID: %d Variant ID: %d Version: %d Build: %d\n",
- info->family_id, info->variant_id, info->version,
- info->build);
+ "Family ID: %u Variant ID: %u Major.Minor.Build: %u.%u.%02X\n",
+ info->family_id, info->variant_id, info->version >> 4,
+ info->version & 0xf, info->build);

dev_info(&client->dev,
- "Matrix X Size: %d Matrix Y Size: %d Object Num: %d\n",
+ "Matrix X Size: %u Matrix Y Size: %u Object Num: %u\n",
info->matrix_xsize, info->matrix_ysize,
info->object_num);

--
1.7.7.3

2012-06-18 04:14:04

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 02/22 v4] Input: atmel_mxt_ts - use client name for irq

The atmel_mxt_ts driver can support multiple devices simultaneously.
Use the i2c_client name instead of the driver name when requesting an
interrupt to make the different interrupts distinguishable in
/proc/interrupts and top.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index eda9eed..cb191fe 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1150,7 +1150,7 @@ static int __devinit mxt_probe(struct i2c_client *client,
goto err_free_object;

error = request_threaded_irq(client->irq, NULL, mxt_interrupt,
- pdata->irqflags, client->dev.driver->name, data);
+ pdata->irqflags, client->name, data);
if (error) {
dev_err(&client->dev, "Failed to register interrupt\n");
goto err_free_object;
--
1.7.7.3

2012-06-18 04:14:01

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 04/22 v4] Input: atmel_mxt_ts - warn if sysfs could not be created

If sysfs entry creation fails, the driver is still usable, so don't
just abort probe. Just warn and continue.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 00b37b1..0c49a1e 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1168,13 +1168,10 @@ static int __devinit mxt_probe(struct i2c_client *client,

error = sysfs_create_group(&client->dev.kobj, &mxt_attr_group);
if (error)
- goto err_unregister_device;
+ dev_warn(&client->dev, "error creating sysfs entries.\n");

return 0;

-err_unregister_device:
- input_unregister_device(input_dev);
- input_dev = NULL;
err_free_irq:
free_irq(client->irq, data);
err_free_object:
--
1.7.7.3

2012-06-18 04:15:06

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH 20/22 v4] Input: atmel_mxt_ts - use T9 reportid range to init number of mt slots

Atmel mxt devices can report one finger for each T9 reportid.
Therefore, this range can be used to report the max number of MT-B slots
to userspace instead of assuming a fixed 10.

Note that mxt_initialized() must complete early, since the input_dev
properties now depend on values in the object table.

Signed-off-by: Daniel Kurtz <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index f5ca237..460c10f 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -212,8 +212,6 @@
/* Touchscreen absolute values */
#define MXT_MAX_AREA 0xff

-#define MXT_MAX_FINGER 10
-
struct mxt_info {
u8 family_id;
u8 variant_id;
@@ -1056,6 +1054,7 @@ static int __devinit mxt_probe(struct i2c_client *client,
struct mxt_data *data;
struct input_dev *input_dev;
int error;
+ unsigned int num_mt_slots;

if (!pdata)
return -EINVAL;
@@ -1082,6 +1081,10 @@ static int __devinit mxt_probe(struct i2c_client *client,

mxt_calc_resolution(data);

+ error = mxt_initialize(data);
+ if (error)
+ goto err_free_object;
+
__set_bit(EV_ABS, input_dev->evbit);
__set_bit(EV_KEY, input_dev->evbit);
__set_bit(BTN_TOUCH, input_dev->keybit);
@@ -1095,9 +1098,10 @@ static int __devinit mxt_probe(struct i2c_client *client,
0, 255, 0, 0);

/* For multi touch */
- error = input_mt_init_slots(input_dev, MXT_MAX_FINGER);
+ num_mt_slots = data->T9_reportid_max - data->T9_reportid_min + 1;
+ error = input_mt_init_slots(input_dev, num_mt_slots);
if (error)
- goto err_free_mem;
+ goto err_free_object;
input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
0, MXT_MAX_AREA, 0, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
@@ -1110,10 +1114,6 @@ static int __devinit mxt_probe(struct i2c_client *client,
input_set_drvdata(input_dev, data);
i2c_set_clientdata(client, data);

- error = mxt_initialize(data);
- if (error)
- goto err_free_object;
-
error = request_threaded_irq(client->irq, NULL, mxt_interrupt,
pdata->irqflags, client->name, data);
if (error) {
--
1.7.7.3

2012-06-18 15:09:57

by Nick Dyer

[permalink] [raw]
Subject: Re: [PATCH 22/22 v4] Input: atmel_mxt_ts - parse T6 reports

Daniel Kurtz wrote:
> The normal messages sent after boot or NVRAM update are T6 reports,
> containing a status, and the config memory checksum. Parse them and dump
> a useful info message.

This can cause a lot of dmesg output - for instance you get 2 messages per
calibration, and the chip can trigger them itself if noise suppression or
anti-touch calibration are enabled. So perhaps dev_dbg()?

--
Nick Dyer
Software Engineer, ITDev Ltd

2012-06-18 15:11:23

by Nick Dyer

[permalink] [raw]
Subject: Re: [PATCH 14/22 v4] Input: atmel_mxt_ts - don't re-read matrix after applying pdata

Daniel Kurtz wrote:
> The matrix x/y size in the Info ID Block represents the number of x/y
> trace lines on the device. There is no need to re-read them after
> applying pdata config, since pdata only configures the object table
> etnries. The matrix size read from the ID can only be updated by a
> firmware update.

This isn't correct. For example, mXT224 can be configured as 16x14, 17x13,
18x12, etc. This only takes effect when the chip is reset, which is why it
is necessary to re-read them after applying the config.

cheers

--
Nick Dyer
Software Engineer, ITDev Ltd
Hardware and Software Development Consultancy
Website: http://www.itdev.co.uk

2012-06-18 15:28:25

by Daniel Kurtz

[permalink] [raw]
Subject: Re: [PATCH 14/22 v4] Input: atmel_mxt_ts - don't re-read matrix after applying pdata

On Mon, Jun 18, 2012 at 11:03 PM, Nick Dyer <[email protected]> wrote:
>
> Daniel Kurtz wrote:
> > The matrix x/y size in the Info ID Block represents the number of x/y
> > trace lines on the device. ?There is no need to re-read them after
> > applying pdata config, since pdata only configures the object table
> > etnries. ?The matrix size read from the ID can only be updated by a
> > firmware update.
>
> This isn't correct. For example, mXT224 can be configured as 16x14, 17x13,
> 18x12, etc. This only takes effect when the chip is reset, which is why it
> is necessary to re-read them after applying the config.

Ah, so is that what T46.1 (Mode) does? I hadn't played with that yet.
Ok, I'll move this patch into the next series which cleans up all of
the configuration stuff.

-Dan

>
>
> cheers
>
> --
> Nick Dyer
> Software Engineer, ITDev Ltd
> Hardware and Software Development Consultancy
> Website: http://www.itdev.co.uk

2012-06-18 15:30:22

by Daniel Kurtz

[permalink] [raw]
Subject: Re: [PATCH 22/22 v4] Input: atmel_mxt_ts - parse T6 reports

On Mon, Jun 18, 2012 at 11:09 PM, Nick Dyer <[email protected]> wrote:
> Daniel Kurtz wrote:
>> The normal messages sent after boot or NVRAM update are T6 reports,
>> containing a status, and the config memory checksum. ?Parse them and dump
>> a useful info message.
>
> This can cause a lot of dmesg output - for instance you get 2 messages per
> calibration, and the chip can trigger them itself if noise suppression or
> anti-touch calibration are enabled. So perhaps dev_dbg()?

Sounds good to me.

>
> --
> Nick Dyer
> Software Engineer, ITDev Ltd

2012-06-18 19:50:05

by Yufeng Shen

[permalink] [raw]
Subject: Re: [PATCH 14/22 v4] Input: atmel_mxt_ts - don't re-read matrix after applying pdata

On Mon, Jun 18, 2012 at 11:03 AM, Nick Dyer <[email protected]> wrote:
> Daniel Kurtz wrote:
>> The matrix x/y size in the Info ID Block represents the number of x/y
>> trace lines on the device. ?There is no need to re-read them after
>> applying pdata config, since pdata only configures the object table
>> etnries. ?The matrix size read from the ID can only be updated by a
>> firmware update.
>
> This isn't correct. For example, mXT224 can be configured as 16x14, 17x13,
> 18x12, etc. This only takes effect when the chip is reset, which is why it
> is necessary to re-read them after applying the config.
>

so look at structure mxt_info

struct mxt_info {
u8 family_id;
u8 variant_id;
u8 version;
u8 build;
u8 matrix_xsize;
u8 matrix_ysize;
u8 object_num;
};

I think it is meant to be corresponding to Information Block of the
device, of which
the matrix_xsize and matrix_ysize mean "The size of the matrix the
device supports",
which should be fixed for a certain chip.

The configurable matrix size is in T9 XSIZE/YSIZE, which, if ever
needed to be read back,
probably read back into something other than mxt_info struct.


> cheers
>
> --
> Nick Dyer
> Software Engineer, ITDev Ltd
> Hardware and Software Development Consultancy
> Website: http://www.itdev.co.uk

2012-06-19 10:26:10

by Nick Dyer

[permalink] [raw]
Subject: Re: [PATCH 14/22 v4] Input: atmel_mxt_ts - don't re-read matrix after applying pdata

Yufeng Shen wrote:
> On Mon, Jun 18, 2012 at 11:03 AM, Nick Dyer <[email protected]> wrote:
>> Daniel Kurtz wrote:
>>> The matrix x/y size in the Info ID Block represents the number of x/y
>>> trace lines on the device. There is no need to re-read them after
>>> applying pdata config, since pdata only configures the object table
>>> etnries. The matrix size read from the ID can only be updated by a
>>> firmware update.
>>
>> This isn't correct. For example, mXT224 can be configured as 16x14, 17x13,
>> 18x12, etc. This only takes effect when the chip is reset, which is why it
>> is necessary to re-read them after applying the config.
>
> so look at structure mxt_info
>
> struct mxt_info {
> u8 family_id;
> u8 variant_id;
> u8 version;
> u8 build;
> u8 matrix_xsize;
> u8 matrix_ysize;
> u8 object_num;
> };
>
> I think it is meant to be corresponding to Information Block of the
> device, of which
> the matrix_xsize and matrix_ysize mean "The size of the matrix the
> device supports",
> which should be fixed for a certain chip.

As I said, it's not fixed, there is a mode setting (T28 byte 2) which
alters the matrix size. This is documented in the protocol guide, if you
have it.

You can also verify this if you want: change mode setting, backup, reset,
and read the information block, you'll see the matrix size changes. I've
just checked that myself on an mXT224.

I also asked the guys at Atmel that I work with, and they confirm that this
behaviour is as designed.

> The configurable matrix size is in T9 XSIZE/YSIZE, which, if ever
> needed to be read back,
> probably read back into something other than mxt_info struct.

The position of the touchscreen within the matrix is a separate
configurable setting, you're confusing the two.

cheers

--
Nick Dyer
Software Engineer, ITDev Ltd
Hardware and Software Development Consultancy
Website: http://www.itdev.co.uk

2012-06-19 15:12:11

by Yufeng Shen

[permalink] [raw]
Subject: Re: [PATCH 14/22 v4] Input: atmel_mxt_ts - don't re-read matrix after applying pdata

On Tue, Jun 19, 2012 at 6:26 AM, Nick Dyer <[email protected]> wrote:
> Yufeng Shen wrote:
>> On Mon, Jun 18, 2012 at 11:03 AM, Nick Dyer <[email protected]> wrote:
>>> Daniel Kurtz wrote:
>>>> The matrix x/y size in the Info ID Block represents the number of x/y
>>>> trace lines on the device. ?There is no need to re-read them after
>>>> applying pdata config, since pdata only configures the object table
>>>> etnries. ?The matrix size read from the ID can only be updated by a
>>>> firmware update.
>>>
>>> This isn't correct. For example, mXT224 can be configured as 16x14, 17x13,
>>> 18x12, etc. This only takes effect when the chip is reset, which is why it
>>> is necessary to re-read them after applying the config.
>>
>> so look at structure mxt_info
>>
>> ?struct mxt_info {
>> ? ? ? ? ? u8 family_id;
>> ? ? ? ? ? u8 variant_id;
>> ? ? ? ? ? u8 version;
>> ? ? ? ? ? u8 build;
>> ? ? ? ? ? u8 matrix_xsize;
>> ? ? ? ? ? u8 matrix_ysize;
>> ? ? ? ? ? u8 object_num;
>> };
>>
>> I think it is meant to be corresponding to Information Block of the
>> device, of which
>> the matrix_xsize and matrix_ysize mean "The size of the matrix the
>> device supports",
>> which should be fixed for a certain chip.
>
> As I said, it's not fixed, there is a mode setting (T28 byte 2) which
> alters the matrix size. This is documented in the protocol guide, if you
> have it.
>
> You can also verify this if you want: change mode setting, backup, reset,
> and read the information block, you'll see the matrix size changes. I've
> just checked that myself on an mXT224.
>

Ah, you are correct. I was looking at the wrong protocol guide. mXT224(E) does
have the mode setting option.

> I also asked the guys at Atmel that I work with, and they confirm that this
> behaviour is as designed.
>
>> The configurable matrix size is in T9 XSIZE/YSIZE, which, if ever
>> needed to be read back,
>> probably read back into something other than mxt_info struct.
>
> The position of the touchscreen within the matrix is a separate
> configurable setting, you're confusing the two.
>
> cheers
>
> --
> Nick Dyer
> Software Engineer, ITDev Ltd
> Hardware and Software Development Consultancy
> Website: http://www.itdev.co.uk

2012-06-20 17:35:54

by Henrik Rydberg

[permalink] [raw]
Subject: Re: [PATCH 00/22 v4] cleanup atmel_mxt_ts

Hi Daniel,

> This patchset cleans up the atmel_mxt_ts touchscreen driver.
>
> These patches mostly contain non-controversial changes that were previously
> reviewed in previous versions of the patchset.

I have queued patches 1-10 locally, review of the others will follow (eventually).

I will push this set to next in a week or so unless Dmitry wants to takes it.

Thanks,
Henrik

2012-06-21 08:41:18

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 01/22 v4] Input: atmel_mxt_ts - set phys to i2c client adapter name

On Mon, Jun 18, 2012 at 12:08:21PM +0800, Daniel Kurtz wrote:
> This allows userspace to more easily distinguish which bus a particular
> atmel_mxt_ts device is attached to.
>
> Signed-off-by: Daniel Kurtz <[email protected]>
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 42e6450..eda9eed 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -1106,6 +1106,7 @@ static int __devinit mxt_probe(struct i2c_client *client,
> }
>
> input_dev->name = "Atmel maXTouch Touchscreen";
> + input_dev->phys = client->adapter->name;

Normally we set phys to 'xxx/input0' nstead of just the adaptor name.

Thanks.

--
Dmitry

2012-06-25 08:12:54

by Henrik Rydberg

[permalink] [raw]
Subject: Re: [PATCH 01/22 v4] Input: atmel_mxt_ts - set phys to i2c client adapter name

On Thu, Jun 21, 2012 at 01:41:07AM -0700, Dmitry Torokhov wrote:
> On Mon, Jun 18, 2012 at 12:08:21PM +0800, Daniel Kurtz wrote:
> > This allows userspace to more easily distinguish which bus a particular
> > atmel_mxt_ts device is attached to.
> >
> > Signed-off-by: Daniel Kurtz <[email protected]>
> > ---
> > drivers/input/touchscreen/atmel_mxt_ts.c | 1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> > index 42e6450..eda9eed 100644
> > --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> > @@ -1106,6 +1106,7 @@ static int __devinit mxt_probe(struct i2c_client *client,
> > }
> >
> > input_dev->name = "Atmel maXTouch Touchscreen";
> > + input_dev->phys = client->adapter->name;
>
> Normally we set phys to 'xxx/input0' nstead of just the adaptor name.

Daniel, will we be seeing another version of this patch?

Thanks,
Henrik

2012-06-25 09:59:11

by Daniel Kurtz

[permalink] [raw]
Subject: Re: [PATCH 01/22 v4] Input: atmel_mxt_ts - set phys to i2c client adapter name

On Mon, Jun 25, 2012 at 4:15 PM, Henrik Rydberg <[email protected]> wrote:
>
> On Thu, Jun 21, 2012 at 01:41:07AM -0700, Dmitry Torokhov wrote:
> > On Mon, Jun 18, 2012 at 12:08:21PM +0800, Daniel Kurtz wrote:
> > > This allows userspace to more easily distinguish which bus a
> > > particular
> > > atmel_mxt_ts device is attached to.
> > >
> > > Signed-off-by: Daniel Kurtz <[email protected]>
> > > ---
> > > ?drivers/input/touchscreen/atmel_mxt_ts.c | ? ?1 +
> > > ?1 files changed, 1 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
> > > b/drivers/input/touchscreen/atmel_mxt_ts.c
> > > index 42e6450..eda9eed 100644
> > > --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> > > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> > > @@ -1106,6 +1106,7 @@ static int __devinit mxt_probe(struct i2c_client
> > > *client,
> > > ? ? }
> > >
> > > ? ? input_dev->name = "Atmel maXTouch Touchscreen";
> > > + ? input_dev->phys = client->adapter->name;
> >
> > Normally we set phys to 'xxx/input0' nstead of just the adaptor name.
>
> Daniel, will we be seeing another version of this patch?

Yes. Any guidance on "xxx" ? atmel_mxt_ts?
What if there are two devices using the same driver?

>
> Thanks,
> Henrik

2012-06-26 01:28:04

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 01/22 v4] Input: atmel_mxt_ts - set phys to i2c client adapter name

On Mon, Jun 25, 2012 at 05:58:46PM +0800, Daniel Kurtz wrote:
> On Mon, Jun 25, 2012 at 4:15 PM, Henrik Rydberg <[email protected]> wrote:
> >
> > On Thu, Jun 21, 2012 at 01:41:07AM -0700, Dmitry Torokhov wrote:
> > > On Mon, Jun 18, 2012 at 12:08:21PM +0800, Daniel Kurtz wrote:
> > > > This allows userspace to more easily distinguish which bus a
> > > > particular
> > > > atmel_mxt_ts device is attached to.
> > > >
> > > > Signed-off-by: Daniel Kurtz <[email protected]>
> > > > ---
> > > > ?drivers/input/touchscreen/atmel_mxt_ts.c | ? ?1 +
> > > > ?1 files changed, 1 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
> > > > b/drivers/input/touchscreen/atmel_mxt_ts.c
> > > > index 42e6450..eda9eed 100644
> > > > --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> > > > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> > > > @@ -1106,6 +1106,7 @@ static int __devinit mxt_probe(struct i2c_client
> > > > *client,
> > > > ? ? }
> > > >
> > > > ? ? input_dev->name = "Atmel maXTouch Touchscreen";
> > > > + ? input_dev->phys = client->adapter->name;
> > >
> > > Normally we set phys to 'xxx/input0' nstead of just the adaptor name.
> >
> > Daniel, will we be seeing another version of this patch?
>
> Yes. Any guidance on "xxx" ? atmel_mxt_ts?

What you have before (client->adapter->name) + "/input0" shoudl work
splendidly.

Thanks.

--
Dmitry

2012-06-27 00:42:12

by Benson Leung

[permalink] [raw]
Subject: Re: [PATCH 01/22 v4] Input: atmel_mxt_ts - set phys to i2c client adapter name

[oops. sorry for duplicate send... resending in plaintext]

Would it more sense to more completely describe the bus type, adapter,
and address in this case, rather than using the client->adapter->name?

Just to give an example from our system, client->adapter->name is
"i915 gmbus panel", so your suggestion would be "i915 gmbus
panel/input0"

How about this instead :
i2c-2-004a/input0

This seems to me more consistent with what is done with USB input
devices, which show a phys field like this :
usb-00:01.2-2.2/input0

Benson

On Mon, Jun 25, 2012 at 6:27 PM, Dmitry Torokhov
<[email protected]> wrote:
>
> On Mon, Jun 25, 2012 at 05:58:46PM +0800, Daniel Kurtz wrote:
> > On Mon, Jun 25, 2012 at 4:15 PM, Henrik Rydberg <[email protected]>
> > wrote:
> > >
> > > On Thu, Jun 21, 2012 at 01:41:07AM -0700, Dmitry Torokhov wrote:
> > > > On Mon, Jun 18, 2012 at 12:08:21PM +0800, Daniel Kurtz wrote:
> > > > > This allows userspace to more easily distinguish which bus a
> > > > > particular
> > > > > atmel_mxt_ts device is attached to.
> > > > >
> > > > > Signed-off-by: Daniel Kurtz <[email protected]>
> > > > > ---
> > > > > ?drivers/input/touchscreen/atmel_mxt_ts.c | ? ?1 +
> > > > > ?1 files changed, 1 insertions(+), 0 deletions(-)
> > > > >
> > > > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
> > > > > b/drivers/input/touchscreen/atmel_mxt_ts.c
> > > > > index 42e6450..eda9eed 100644
> > > > > --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> > > > > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> > > > > @@ -1106,6 +1106,7 @@ static int __devinit mxt_probe(struct
> > > > > i2c_client
> > > > > *client,
> > > > > ? ? }
> > > > >
> > > > > ? ? input_dev->name = "Atmel maXTouch Touchscreen";
> > > > > + ? input_dev->phys = client->adapter->name;
> > > >
> > > > Normally we set phys to 'xxx/input0' nstead of just the adaptor
> > > > name.
> > >
> > > Daniel, will we be seeing another version of this patch?
> >
> > Yes. ?Any guidance on "xxx" ? ?atmel_mxt_ts?
>
> What you have before (client->adapter->name) + "/input0" shoudl work
> splendidly.
>
> Thanks.
>
> --
> Dmitry




--
Benson Leung
Software Engineer,?Chrom* OS
[email protected]