-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 04/04/2012 09:10 PM, Dmitry Torokhov wrote:
[suggestions for my patch]
Based on your input I have prepared two patches which might be easier to
review than the complete diff.
The first one is IMHO pretty straightforward and implements most of the
small bits.
The second one sits on top of the first one and is an attempt at more
generic attribute handling. I must say that I am not too happy about it,
but maybe I am thinking too convoluted here. It saves about 3 lines of
code and exchanges a lot of easy and straightforward code with stuff
that is harder to follow - especially the lookup of the matching
attribute makes me unhappy. There are bits in it I like and will
probably use. But the lookup...
any suggestions are welcome.
Bye,
Simon
- --
Simon Budig kernel concepts GmbH
simon.budig@xxxxxxxxxxxxxxxxx Sieghuetter Hauptweg 48
+49-271-771091-17 D-57072 Siegen
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk99lfoACgkQO2O/RXesiHAE6gCgw2EK5b6MKzfYY7ekEwdNy4FG
GlUAn2db538XupC6i7OZoP06Zb+RFaBI
=UTET
-----END PGP SIGNATURE-----
commit c4429d03fd6bffd51eb5b47dfb173ecd6c015e10
Author: Simon Budig <simon.budig@xxxxxxxxxxxxxxxxx>
Date: Wed Apr 4 23:21:51 2012 +0200
incorporate a set of fixes from Dmitry Torokhov
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 58e35c3..d96eb8f 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -62,6 +62,7 @@ struct edt_ft5x06_i2c_ts_data {
int gain;
int offset;
int report_rate;
+ char name[24];
};
static int edt_ft5x06_ts_readwrite(struct i2c_client *client,
@@ -102,7 +103,7 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
struct edt_ft5x06_i2c_ts_data *tsdata = dev_id;
unsigned char touching = 0;
unsigned char rdbuf[26], wrbuf[1];
- int i, have_abs, type, ret;
+ int i, have_abs, type, x, y, id, ret;
memset(wrbuf, 0, sizeof(wrbuf));
memset(rdbuf, 0, sizeof(rdbuf));
@@ -126,7 +127,7 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
rdbuf[0], rdbuf[1], rdbuf[2]);
}
- have_abs = 0;
+ have_abs = false;
touching = rdbuf[3];
for (i = 0; i < touching; i++) {
type = rdbuf[i*4+5] >> 6;
@@ -134,23 +135,20 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
if (type == 0x01 || type == 0x03)
continue;
+ x = ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff;
+ y = ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff;
+ id = (rdbuf[i*4+7] >> 4) & 0x0f;
+
if (!have_abs) {
input_report_key(tsdata->input, BTN_TOUCH, 1);
input_report_abs(tsdata->input, ABS_PRESSURE, 1);
- input_report_abs(tsdata->input, ABS_X,
- ((rdbuf[i*4+5] << 8) |
- rdbuf[i*4+6]) & 0x0fff);
- input_report_abs(tsdata->input, ABS_Y,
- ((rdbuf[i*4+7] << 8) |
- rdbuf[i*4+8]) & 0x0fff);
- have_abs = 1;
+ input_report_abs(tsdata->input, ABS_X, x);
+ input_report_abs(tsdata->input, ABS_Y, y);
+ have_abs = true;
}
- input_report_abs(tsdata->input, ABS_MT_POSITION_X,
- ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff);
- input_report_abs(tsdata->input, ABS_MT_POSITION_Y,
- ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff);
- input_report_abs(tsdata->input, ABS_MT_TRACKING_ID,
- (rdbuf[i*4+7] >> 4) & 0x0f);
+ input_report_abs(tsdata->input, ABS_MT_POSITION_X, x);
+ input_report_abs(tsdata->input, ABS_MT_POSITION_Y, y);
+ input_report_abs(tsdata->input, ABS_MT_TRACKING_ID, id);
input_mt_sync(tsdata->input);
}
if (!have_abs) {
@@ -378,7 +376,7 @@ static ssize_t edt_ft5x06_i2c_mode_store(struct device *dev,
dev_err(dev, "failed to switch to factory mode (%d)\n",
ret);
} else {
- tsdata->factory_mode = 1;
+ tsdata->factory_mode = true;
for (i = 0; i < 10; i++) {
mdelay(5);
/* mode register is 0x01 when in factory mode */
@@ -400,7 +398,7 @@ static ssize_t edt_ft5x06_i2c_mode_store(struct device *dev,
dev_err(dev, "failed to switch to work mode (%d)\n",
ret);
} else {
- tsdata->factory_mode = 0;
+ tsdata->factory_mode = false;
for (i = 0; i < 10; i++) {
mdelay(5);
/* mode register is 0x01 when in factory mode */
@@ -515,8 +513,8 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
+ const struct edt_ft5x06_platform_data *pdata = client->dev.platform_data;
struct edt_ft5x06_i2c_ts_data *tsdata;
- struct edt_ft5x06_platform_data *pdata;
struct input_dev *input;
int error;
u8 rdbuf[23];
@@ -524,7 +522,7 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
dev_dbg(&client->dev, "probing for EDT FT5x06 I2C\n");
- if (!client->dev.platform_data) {
+ if (!pdata) {
dev_err(&client->dev, "no platform data?\n");
return -ENODEV;
}
@@ -538,7 +536,6 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
dev_set_drvdata(&client->dev, tsdata);
tsdata->client = client;
- pdata = client->dev.platform_data;
tsdata->reset_pin = pdata->reset_pin;
mutex_init(&tsdata->mutex);
@@ -581,7 +578,7 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
mutex_lock(&tsdata->mutex);
- tsdata->factory_mode = 0;
+ tsdata->factory_mode = false;
if (edt_ft5x06_ts_readwrite(client, 1, "\xbb", 22, rdbuf) < 0) {
dev_err(&client->dev, "probing failed\n");
@@ -622,6 +619,7 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
} else {
dev_info(&client->dev, "Product ID \"%s\"\n", model_name);
}
+ strncpy (tsdata->name, model_name, sizeof (tsdata->name) - 1);
input = input_allocate_device();
if (!input) {
@@ -643,7 +641,7 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
0, tsdata->num_y * 64 - 1, 0, 0);
input_set_abs_params(input, ABS_MT_TRACKING_ID, 0, 15, 0, 0);
- input->name = kstrdup(model_name, GFP_NOIO);
+ input->name = tsdata->name;
input->id.bustype = BUS_I2C;
input->dev.parent = &client->dev;
@@ -659,7 +657,6 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
client->name, tsdata)) {
dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
- input = NULL;
error = -ENOMEM;
goto err_unregister_device;
}
@@ -683,10 +680,8 @@ err_unregister_device:
input_unregister_device(input);
input = NULL;
err_free_input_device:
- if (input) {
- kfree(input->name);
+ if (input)
input_free_device(input);
- }
err_free_irq_pin:
gpio_free(tsdata->irq_pin);
err_free_reset_pin:
@@ -704,7 +699,6 @@ static int edt_ft5x06_i2c_ts_remove(struct i2c_client *client)
sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_i2c_attr_group);
free_irq(tsdata->irq, tsdata);
- kfree(tsdata->input->name);
input_unregister_device(tsdata->input);
gpio_free(tsdata->irq_pin);
if (tsdata->reset_pin >= 0)
@@ -714,6 +708,7 @@ static int edt_ft5x06_i2c_ts_remove(struct i2c_client *client)
return 0;
}
+#ifdef CONFIG_PM_SLEEP
static int edt_ft5x06_i2c_ts_suspend(struct device *dev)
{
struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
@@ -733,6 +728,7 @@ static int edt_ft5x06_i2c_ts_resume(struct device *dev)
return 0;
}
+#endif
static SIMPLE_DEV_PM_OPS(edt_ft5x06_i2c_ts_pm_ops,
edt_ft5x06_i2c_ts_suspend, edt_ft5x06_i2c_ts_resume);
commit c79729c84cbdde766517a863ade907d8e9a830d6
Author: Simon Budig <simon.budig@xxxxxxxxxxxxxxxxx>
Date: Thu Apr 5 14:43:18 2012 +0200
rework attribute handling. Not sure if this improves things
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index d96eb8f..9e5900c 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -35,7 +35,7 @@
#include <linux/input/edt-ft5x06.h>
-#define DRIVER_VERSION "v0.5"
+#define DRIVER_VERSION "v0.6"
#define WORK_REGISTER_THRESHOLD 0x00
#define WORK_REGISTER_REPORT_RATE 0x08
@@ -47,6 +47,14 @@
#define WORK_REGISTER_OPMODE 0x3c
#define FACTORY_REGISTER_OPMODE 0x01
+enum edt_ft5x06_setting {
+ SETTING_THRESHOLD,
+ SETTING_GAIN,
+ SETTING_OFFSET,
+ SETTING_REPORT_RATE,
+ N_SETTINGS, /* must be last entry */
+};
+
struct edt_ft5x06_i2c_ts_data {
struct i2c_client *client;
struct input_dev *input;
@@ -58,13 +66,69 @@ struct edt_ft5x06_i2c_ts_data {
struct mutex mutex;
bool factory_mode;
- int threshold;
- int gain;
- int offset;
- int report_rate;
+ int settings[N_SETTINGS];
char name[24];
};
+struct edt_ft5x06_settings_data_entry {
+ u8 addr;
+ u8 min;
+ u8 max;
+};
+
+static const struct edt_ft5x06_settings_data_entry
+edt_ft5x06_settings_data[N_SETTINGS] = {
+ [SETTING_THRESHOLD] = { WORK_REGISTER_THRESHOLD, 20, 80 },
+ [SETTING_GAIN] = { WORK_REGISTER_GAIN, 0, 31 },
+ [SETTING_OFFSET] = { WORK_REGISTER_OFFSET, 0, 31 },
+ [SETTING_REPORT_RATE] = { WORK_REGISTER_REPORT_RATE, 3, 14 },
+};
+
+
+static ssize_t edt_ft5x06_i2c_setting_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf);
+static ssize_t edt_ft5x06_i2c_setting_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count);
+static ssize_t edt_ft5x06_i2c_mode_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf);
+static ssize_t edt_ft5x06_i2c_mode_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count);
+static ssize_t edt_ft5x06_i2c_raw_data_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf);
+
+static DEVICE_ATTR(gain, 0664,
+ edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(offset, 0664,
+ edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(threshold, 0664,
+ edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(report_rate, 0664,
+ edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(mode, 0664,
+ edt_ft5x06_i2c_mode_show, edt_ft5x06_i2c_mode_store);
+static DEVICE_ATTR(raw_data, 0444,
+ edt_ft5x06_i2c_raw_data_show, NULL);
+
+static struct attribute *edt_ft5x06_i2c_attrs[] = {
+ [SETTING_GAIN] = &dev_attr_gain.attr,
+ [SETTING_OFFSET] = &dev_attr_offset.attr,
+ [SETTING_THRESHOLD] = &dev_attr_threshold.attr,
+ [SETTING_REPORT_RATE] = &dev_attr_report_rate.attr,
+ &dev_attr_mode.attr,
+ &dev_attr_raw_data.attr,
+ NULL
+};
+
+static const struct attribute_group edt_ft5x06_i2c_attr_group = {
+ .attrs = edt_ft5x06_i2c_attrs,
+};
+
+
static int edt_ft5x06_ts_readwrite(struct i2c_client *client,
u16 wr_len, u8 *wr_buf,
u16 rd_len, u8 *rd_buf)
@@ -217,27 +281,14 @@ static ssize_t edt_ft5x06_i2c_setting_show(struct device *dev,
struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
struct i2c_client *client = tsdata->client;
int ret = 0;
- int *value;
- u8 addr;
+ int i;
- switch (attr->attr.name[0]) {
- case 't': /* threshold */
- addr = WORK_REGISTER_THRESHOLD;
- value = &tsdata->threshold;
- break;
- case 'g': /* gain */
- addr = WORK_REGISTER_GAIN;
- value = &tsdata->gain;
- break;
- case 'o': /* offset */
- addr = WORK_REGISTER_OFFSET;
- value = &tsdata->offset;
- break;
- case 'r': /* report rate */
- addr = WORK_REGISTER_REPORT_RATE;
- value = &tsdata->report_rate;
- break;
- default:
+ for (i = 0; i < N_SETTINGS; i++) {
+ if (edt_ft5x06_i2c_attrs[i] == &attr->attr)
+ break;
+ }
+
+ if (i == N_SETTINGS) {
dev_err(&client->dev,
"unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
attr->attr.name);
@@ -253,20 +304,21 @@ static ssize_t edt_ft5x06_i2c_setting_show(struct device *dev,
return -EIO;
}
- ret = edt_ft5x06_i2c_register_read(tsdata, addr);
+ ret = edt_ft5x06_i2c_register_read(tsdata,
+ edt_ft5x06_settings_data[i].addr);
if (ret < 0) {
dev_err(&tsdata->client->dev,
- "Unable to write to i2c touchscreen!\n");
+ "Unable to read from i2c touchscreen!\n");
mutex_unlock(&tsdata->mutex);
return ret;
}
mutex_unlock(&tsdata->mutex);
- if (ret != *value) {
+ if (ret != tsdata->settings[i]) {
dev_info(&tsdata->client->dev,
"i2c read (%d) and stored value (%d) differ. Huh?\n",
- ret, *value);
- *value = ret;
+ ret, tsdata->settings[i]);
+ tsdata->settings[i] = ret;
}
return sprintf(buf, "%d\n", ret);
@@ -278,10 +330,21 @@ static ssize_t edt_ft5x06_i2c_setting_store(struct device *dev,
{
struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
struct i2c_client *client = tsdata->client;
- int ret = 0;
- u8 addr;
+ int i, ret = 0;
unsigned int val;
+ for (i = 0; i < N_SETTINGS; i++) {
+ if (edt_ft5x06_i2c_attrs[i] == &attr->attr)
+ break;
+ }
+
+ if (i == N_SETTINGS) {
+ dev_err(&client->dev,
+ "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
+ attr->attr.name);
+ return -EINVAL;
+ }
+
mutex_lock(&tsdata->mutex);
if (tsdata->factory_mode) {
@@ -298,36 +361,14 @@ static ssize_t edt_ft5x06_i2c_setting_store(struct device *dev,
goto out;
}
- switch (attr->attr.name[0]) {
- case 't': /* threshold */
- addr = WORK_REGISTER_THRESHOLD;
- val = val < 20 ? 20 : val > 80 ? 80 : val;
- tsdata->threshold = val;
- break;
- case 'g': /* gain */
- addr = WORK_REGISTER_GAIN;
- val = val < 0 ? 0 : val > 31 ? 31 : val;
- tsdata->gain = val;
- break;
- case 'o': /* offset */
- addr = WORK_REGISTER_OFFSET;
- val = val < 0 ? 0 : val > 31 ? 31 : val;
- tsdata->offset = val;
- break;
- case 'r': /* report rate */
- addr = WORK_REGISTER_REPORT_RATE;
- val = val < 3 ? 3 : val > 14 ? 14 : val;
- tsdata->report_rate = val;
- break;
- default:
- dev_err(&client->dev,
- "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
- attr->attr.name);
- ret = -EINVAL;
- goto out;
- }
-
- ret = edt_ft5x06_i2c_register_write(tsdata, addr, val);
+ if (val < edt_ft5x06_settings_data[i].min)
+ val = edt_ft5x06_settings_data[i].min;
+ else if (val > edt_ft5x06_settings_data[i].max)
+ val = edt_ft5x06_settings_data[i].max;
+ tsdata->settings[i] = val;
+ ret = edt_ft5x06_i2c_register_write(tsdata,
+ edt_ft5x06_settings_data[i].addr,
+ val);
if (ret < 0) {
dev_err(&tsdata->client->dev,
@@ -411,18 +452,11 @@ static ssize_t edt_ft5x06_i2c_mode_store(struct device *dev,
i*5);
/* restore parameters */
- edt_ft5x06_i2c_register_write(tsdata,
- WORK_REGISTER_THRESHOLD,
- tsdata->threshold);
- edt_ft5x06_i2c_register_write(tsdata,
- WORK_REGISTER_GAIN,
- tsdata->gain);
- edt_ft5x06_i2c_register_write(tsdata,
- WORK_REGISTER_OFFSET,
- tsdata->offset);
- edt_ft5x06_i2c_register_write(tsdata,
- WORK_REGISTER_REPORT_RATE,
- tsdata->report_rate);
+ for (i = 0; i < N_SETTINGS; i++) {
+ edt_ft5x06_i2c_register_write(tsdata,
+ edt_ft5x06_settings_data[i].addr,
+ tsdata->settings[i]);
+ }
enable_irq(tsdata->irq);
}
@@ -482,33 +516,6 @@ static ssize_t edt_ft5x06_i2c_raw_data_show(struct device *dev,
}
-static DEVICE_ATTR(gain, 0664,
- edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
-static DEVICE_ATTR(offset, 0664,
- edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
-static DEVICE_ATTR(threshold, 0664,
- edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
-static DEVICE_ATTR(report_rate, 0664,
- edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
-static DEVICE_ATTR(mode, 0664,
- edt_ft5x06_i2c_mode_show, edt_ft5x06_i2c_mode_store);
-static DEVICE_ATTR(raw_data, 0444,
- edt_ft5x06_i2c_raw_data_show, NULL);
-
-static struct attribute *edt_ft5x06_i2c_attrs[] = {
- &dev_attr_gain.attr,
- &dev_attr_offset.attr,
- &dev_attr_threshold.attr,
- &dev_attr_report_rate.attr,
- &dev_attr_mode.attr,
- &dev_attr_raw_data.attr,
- NULL
-};
-
-static const struct attribute_group edt_ft5x06_i2c_attr_group = {
- .attrs = edt_ft5x06_i2c_attrs,
-};
-
static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -516,7 +523,7 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
const struct edt_ft5x06_platform_data *pdata = client->dev.platform_data;
struct edt_ft5x06_i2c_ts_data *tsdata;
struct input_dev *input;
- int error;
+ int i, error;
u8 rdbuf[23];
char *model_name, *fw_version;
@@ -586,14 +593,10 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
goto err_free_irq_pin;
}
- tsdata->threshold = edt_ft5x06_i2c_register_read(tsdata,
- WORK_REGISTER_THRESHOLD);
- tsdata->gain = edt_ft5x06_i2c_register_read(tsdata,
- WORK_REGISTER_GAIN);
- tsdata->offset = edt_ft5x06_i2c_register_read(tsdata,
- WORK_REGISTER_OFFSET);
- tsdata->report_rate = edt_ft5x06_i2c_register_read(tsdata,
- WORK_REGISTER_REPORT_RATE);
+ for (i = 0; i < N_SETTINGS; i++) {
+ tsdata->settings[i] = edt_ft5x06_i2c_register_read(tsdata,
+ edt_ft5x06_settings_data[i].addr);
+ }
tsdata->num_x = edt_ft5x06_i2c_register_read(tsdata,
WORK_REGISTER_NUM_X);
tsdata->num_y = edt_ft5x06_i2c_register_read(tsdata,
