* [PATCH 0/6] input: elants: Support Asus TF300T touchscreen
@ 2019-12-10 0:19 Michał Mirosław
2019-12-10 0:19 ` [PATCH 2/6] input: elants: support old touch report format Michał Mirosław
` (6 more replies)
0 siblings, 7 replies; 19+ messages in thread
From: Michał Mirosław @ 2019-12-10 0:19 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Dmitry Osipenko, linux-kernel
This series implements changes needed to support EKTF3624-based touchscreen
used in eg. Asus Transformer Pad TF300T.
Michał Mirosław (6):
input: elants: document some registers and values
input: elants: support old touch report format
input: elants: support common touchscreen DT properties
input: elants: detect max_x/y from hardware
input: elants: refactor elants_i2c_execute_command()
input: elants: read touchscreen size for EKTF3624
drivers/input/touchscreen/elants_i2c.c | 365 ++++++++++++++++---------
1 file changed, 237 insertions(+), 128 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/6] input: elants: support old touch report format
2019-12-10 0:19 [PATCH 0/6] input: elants: Support Asus TF300T touchscreen Michał Mirosław
@ 2019-12-10 0:19 ` Michał Mirosław
2019-12-10 0:19 ` [PATCH 3/6] input: elants: support common touchscreen DT properties Michał Mirosław
` (5 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2019-12-10 0:19 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, linux-kernel, Dmitry Osipenko
Support ELAN touchpad sensor with older firmware as found on eg. Asus
Transformer Pads.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/input/touchscreen/elants_i2c.c | 36 ++++++++++++++++++--------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
index 887888c53996..eadd26d5a06f 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -65,6 +65,7 @@
#define CMD_HEADER_REK 0x66
/* FW position data */
+#define PACKET_SIZE_OLD 40
#define PACKET_SIZE 55
#define MAX_CONTACT_NUM 10
#define FW_POS_HEADER 0
@@ -792,7 +793,8 @@ static int elants_i2c_fw_update(struct elants_data *ts)
* Event reporting.
*/
-static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf)
+static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf,
+ size_t report_len)
{
struct input_dev *input = ts->input;
unsigned int n_fingers;
@@ -804,7 +806,8 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf)
buf[FW_POS_STATE];
dev_dbg(&ts->client->dev,
- "n_fingers: %u, state: %04x\n", n_fingers, finger_state);
+ "n_fingers: %u, state: %04x, report_len: %zu\n",
+ n_fingers, finger_state, report_len);
for (i = 0; i < MAX_CONTACT_NUM && n_fingers; i++) {
if (finger_state & 1) {
@@ -814,8 +817,16 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf)
pos = &buf[FW_POS_XY + i * 3];
x = (((u16)pos[0] & 0xf0) << 4) | pos[1];
y = (((u16)pos[0] & 0x0f) << 8) | pos[2];
- p = buf[FW_POS_PRESSURE + i];
- w = buf[FW_POS_WIDTH + i];
+ if (report_len == PACKET_SIZE_OLD) {
+ w = buf[FW_POS_WIDTH + i / 2];
+ w >>= 4 * (~i & 1); // little-endian-nibbles
+ w |= w << 4;
+ w |= !w;
+ p = w;
+ } else {
+ p = buf[FW_POS_PRESSURE + i];
+ w = buf[FW_POS_WIDTH + i];
+ }
dev_dbg(&ts->client->dev, "i=%d x=%d y=%d p=%d w=%d\n",
i, x, y, p, w);
@@ -848,7 +859,8 @@ static u8 elants_i2c_calculate_checksum(u8 *buf)
return checksum;
}
-static void elants_i2c_event(struct elants_data *ts, u8 *buf)
+static void elants_i2c_event(struct elants_data *ts, u8 *buf,
+ size_t report_len)
{
u8 checksum = elants_i2c_calculate_checksum(buf);
@@ -862,7 +874,7 @@ static void elants_i2c_event(struct elants_data *ts, u8 *buf)
"%s: unknown packet type: %02x\n",
__func__, buf[FW_POS_HEADER]);
else
- elants_i2c_mt_event(ts, buf);
+ elants_i2c_mt_event(ts, buf, report_len);
}
static irqreturn_t elants_i2c_irq(int irq, void *_dev)
@@ -920,7 +932,8 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
break;
case QUEUE_HEADER_SINGLE:
- elants_i2c_event(ts, &ts->buf[HEADER_SIZE]);
+ elants_i2c_event(ts, &ts->buf[HEADER_SIZE],
+ ts->buf[FW_HDR_LENGTH]);
break;
case QUEUE_HEADER_NORMAL:
@@ -933,17 +946,18 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
}
report_len = ts->buf[FW_HDR_LENGTH] / report_count;
- if (report_len != PACKET_SIZE) {
+ if (report_len != PACKET_SIZE &&
+ report_len != PACKET_SIZE_OLD) {
dev_err(&client->dev,
- "mismatching report length: %*ph\n",
+ "unsupported report length: %*ph\n",
HEADER_SIZE, ts->buf);
break;
}
for (i = 0; i < report_count; i++) {
u8 *buf = ts->buf + HEADER_SIZE +
- i * PACKET_SIZE;
- elants_i2c_event(ts, buf);
+ i * report_len;
+ elants_i2c_event(ts, buf, report_len);
}
break;
--
2.20.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 1/6] input: elants: document some registers and values
2019-12-10 0:19 [PATCH 0/6] input: elants: Support Asus TF300T touchscreen Michał Mirosław
2019-12-10 0:19 ` [PATCH 2/6] input: elants: support old touch report format Michał Mirosław
2019-12-10 0:19 ` [PATCH 3/6] input: elants: support common touchscreen DT properties Michał Mirosław
@ 2019-12-10 0:19 ` Michał Mirosław
2019-12-10 0:19 ` [PATCH 6/6] input: elants: read touchscreen size for EKTF3624 Michał Mirosław
` (3 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2019-12-10 0:19 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, linux-kernel, Dmitry Osipenko
Add information found in downstream kernels, to make the code less
magic.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/input/touchscreen/elants_i2c.c | 29 +++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
index d4ad24ea54c8..887888c53996 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -77,7 +77,11 @@
#define HEADER_REPORT_10_FINGER 0x62
-/* Header (4 bytes) plus 3 fill 10-finger packets */
+/* Power state */
+#define PWR_STATE_DEEP_SLEEP 0
+#define PWR_STATE_NORMAL 1
+
+/* Header (4 bytes) plus 3 full 10-finger packets */
#define MAX_PACKET_SIZE 169
#define BOOT_TIME_DELAY_MS 50
@@ -87,10 +91,21 @@
#define E_ELAN_INFO_BC_VER 0x10
#define E_ELAN_INFO_TEST_VER 0xE0
#define E_ELAN_INFO_FW_ID 0xF0
+#define E_POWER_MODE 0x40
+#define E_POWER_STATE 0x50
+#define E_INFO_X_RES 0x60
+#define E_INFO_Y_RES 0x63
#define E_INFO_OSR 0xD6
#define E_INFO_PHY_SCAN 0xD7
#define E_INFO_PHY_DRIVER 0xD8
+/* FW write command, 0x54 0x?? 0x0, 0x01 */
+#define E_POWER_MODE_BATTERY 0x40
+#define E_POWER_MODE_AC 0x41
+#define E_POWER_MODE_USB 0x42
+#define E_POWER_STATE_SLEEP 0x50
+#define E_POWER_STATE_RESUME 0x58
+
#define MAX_RETRIES 3
#define MAX_FW_UPDATE_RETRIES 30
@@ -231,8 +246,8 @@ static int elants_i2c_calibrate(struct elants_data *ts)
{
struct i2c_client *client = ts->client;
int ret, error;
- static const u8 w_flashkey[] = { 0x54, 0xC0, 0xE1, 0x5A };
- static const u8 rek[] = { 0x54, 0x29, 0x00, 0x01 };
+ static const u8 w_flashkey[] = { CMD_HEADER_WRITE, 0xC0, 0xE1, 0x5A };
+ static const u8 rek[] = { CMD_HEADER_WRITE, 0x29, 0x00, 0x01 };
static const u8 rek_resp[] = { CMD_HEADER_REK, 0x66, 0x66, 0x66 };
disable_irq(client->irq);
@@ -1295,7 +1310,9 @@ static int __maybe_unused elants_i2c_suspend(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct elants_data *ts = i2c_get_clientdata(client);
- const u8 set_sleep_cmd[] = { 0x54, 0x50, 0x00, 0x01 };
+ const u8 set_sleep_cmd[] = {
+ CMD_HEADER_WRITE, E_POWER_STATE_SLEEP, 0x00, 0x01
+ };
int retry_cnt;
int error;
@@ -1332,7 +1349,9 @@ static int __maybe_unused elants_i2c_resume(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct elants_data *ts = i2c_get_clientdata(client);
- const u8 set_active_cmd[] = { 0x54, 0x58, 0x00, 0x01 };
+ const u8 set_active_cmd[] = {
+ CMD_HEADER_WRITE, E_POWER_STATE_RESUME, 0x00, 0x01
+ };
int retry_cnt;
int error;
--
2.20.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/6] input: elants: support common touchscreen DT properties
2019-12-10 0:19 [PATCH 0/6] input: elants: Support Asus TF300T touchscreen Michał Mirosław
2019-12-10 0:19 ` [PATCH 2/6] input: elants: support old touch report format Michał Mirosław
@ 2019-12-10 0:19 ` Michał Mirosław
2019-12-10 1:03 ` Dmitry Osipenko
2019-12-10 0:19 ` [PATCH 1/6] input: elants: document some registers and values Michał Mirosław
` (4 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Michał Mirosław @ 2019-12-10 0:19 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, linux-kernel, Dmitry Osipenko
Support common DT properties like axis inversions to complement
information obtained from device's firmware.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/input/touchscreen/elants_i2c.c | 27 ++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
index eadd26d5a06f..02bd5e3e2171 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -31,6 +31,7 @@
#include <linux/buffer_head.h>
#include <linux/slab.h>
#include <linux/firmware.h>
+#include <linux/input/touchscreen.h>
#include <linux/input/mt.h>
#include <linux/acpi.h>
#include <linux/of.h>
@@ -146,8 +147,7 @@ struct elants_data {
u16 hw_version;
unsigned int x_res; /* resolution in units/mm */
unsigned int y_res;
- unsigned int x_max;
- unsigned int y_max;
+ struct touchscreen_properties prop;
enum elants_state state;
enum elants_iap_mode iap_mode;
@@ -498,10 +498,10 @@ static int elants_i2c_query_ts_info(struct elants_data *ts)
rows, cols, osr);
} else {
/* translate trace number to TS resolution */
- ts->x_max = ELAN_TS_RESOLUTION(rows, osr);
- ts->x_res = DIV_ROUND_CLOSEST(ts->x_max, phy_x);
- ts->y_max = ELAN_TS_RESOLUTION(cols, osr);
- ts->y_res = DIV_ROUND_CLOSEST(ts->y_max, phy_y);
+ ts->prop.max_x = ELAN_TS_RESOLUTION(rows, osr);
+ ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
+ ts->prop.max_y = ELAN_TS_RESOLUTION(cols, osr);
+ ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
}
return 0;
@@ -833,8 +833,7 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf,
input_mt_slot(input, i);
input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
- input_event(input, EV_ABS, ABS_MT_POSITION_X, x);
- input_event(input, EV_ABS, ABS_MT_POSITION_Y, y);
+ touchscreen_report_pos(input, &ts->prop, x, y, true);
input_event(input, EV_ABS, ABS_MT_PRESSURE, p);
input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, w);
@@ -1251,13 +1250,15 @@ static int elants_i2c_probe(struct i2c_client *client,
ts->input->name = "Elan Touchscreen";
ts->input->id.bustype = BUS_I2C;
+ touchscreen_parse_properties(ts->input, true, &ts->prop);
+
__set_bit(BTN_TOUCH, ts->input->keybit);
__set_bit(EV_ABS, ts->input->evbit);
__set_bit(EV_KEY, ts->input->evbit);
/* Single touch input params setup */
- input_set_abs_params(ts->input, ABS_X, 0, ts->x_max, 0, 0);
- input_set_abs_params(ts->input, ABS_Y, 0, ts->y_max, 0, 0);
+ input_set_abs_params(ts->input, ABS_X, 0, ts->prop.max_x, 0, 0);
+ input_set_abs_params(ts->input, ABS_Y, 0, ts->prop.max_y, 0, 0);
input_set_abs_params(ts->input, ABS_PRESSURE, 0, 255, 0, 0);
input_abs_set_res(ts->input, ABS_X, ts->x_res);
input_abs_set_res(ts->input, ABS_Y, ts->y_res);
@@ -1271,8 +1272,10 @@ static int elants_i2c_probe(struct i2c_client *client,
return error;
}
- input_set_abs_params(ts->input, ABS_MT_POSITION_X, 0, ts->x_max, 0, 0);
- input_set_abs_params(ts->input, ABS_MT_POSITION_Y, 0, ts->y_max, 0, 0);
+ input_set_abs_params(ts->input, ABS_MT_POSITION_X,
+ 0, ts->prop.max_x, 0, 0);
+ input_set_abs_params(ts->input, ABS_MT_POSITION_Y,
+ 0, ts->prop.max_y, 0, 0);
input_set_abs_params(ts->input, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
input_set_abs_params(ts->input, ABS_MT_PRESSURE, 0, 255, 0, 0);
input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
--
2.20.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 5/6] input: elants: refactor elants_i2c_execute_command()
2019-12-10 0:19 [PATCH 0/6] input: elants: Support Asus TF300T touchscreen Michał Mirosław
` (3 preceding siblings ...)
2019-12-10 0:19 ` [PATCH 6/6] input: elants: read touchscreen size for EKTF3624 Michał Mirosław
@ 2019-12-10 0:19 ` Michał Mirosław
2019-12-10 1:34 ` Dmitry Osipenko
2019-12-10 0:19 ` [PATCH 4/6] input: elants: detect max_x/y from hardware Michał Mirosław
2019-12-10 0:59 ` [PATCH 0/6] input: elants: Support Asus TF300T touchscreen Dmitry Osipenko
6 siblings, 1 reply; 19+ messages in thread
From: Michał Mirosław @ 2019-12-10 0:19 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, linux-kernel, Dmitry Osipenko
Apply some DRY-ing to elants_i2c_execute_command() callers.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/input/touchscreen/elants_i2c.c | 182 +++++++++++++------------
1 file changed, 93 insertions(+), 89 deletions(-)
diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
index 2e6c9aa60496..27aca3971da5 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -201,7 +201,8 @@ static int elants_i2c_read(struct i2c_client *client, void *data, size_t size)
static int elants_i2c_execute_command(struct i2c_client *client,
const u8 *cmd, size_t cmd_size,
- u8 *resp, size_t resp_size)
+ u8 *resp, size_t resp_size,
+ int retries, const char *cmd_name)
{
struct i2c_msg msgs[2];
int ret;
@@ -217,30 +218,55 @@ static int elants_i2c_execute_command(struct i2c_client *client,
break;
default:
- dev_err(&client->dev, "%s: invalid command %*ph\n",
- __func__, (int)cmd_size, cmd);
+ dev_err(&client->dev, "(%s): invalid command: %*ph\n",
+ cmd_name, (int)cmd_size, cmd);
return -EINVAL;
}
- msgs[0].addr = client->addr;
- msgs[0].flags = client->flags & I2C_M_TEN;
- msgs[0].len = cmd_size;
- msgs[0].buf = (u8 *)cmd;
+ for (;;) {
+ msgs[0].addr = client->addr;
+ msgs[0].flags = client->flags & I2C_M_TEN;
+ msgs[0].len = cmd_size;
+ msgs[0].buf = (u8 *)cmd;
- msgs[1].addr = client->addr;
- msgs[1].flags = client->flags & I2C_M_TEN;
- msgs[1].flags |= I2C_M_RD;
- msgs[1].len = resp_size;
- msgs[1].buf = resp;
+ msgs[1].addr = client->addr;
+ msgs[1].flags = client->flags & I2C_M_TEN;
+ msgs[1].flags |= I2C_M_RD;
+ msgs[1].len = resp_size;
+ msgs[1].buf = resp;
- ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
- if (ret < 0)
- return ret;
+ ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+ if (ret < 0) {
+ if (--retries > 0) {
+ dev_dbg(&client->dev,
+ "(%s) I2C transfer failed: %d (retrying)\n",
+ cmd_name, ret);
+ continue;
+ }
- if (ret != ARRAY_SIZE(msgs) || resp[FW_HDR_TYPE] != expected_response)
- return -EIO;
+ dev_err(&client->dev,
+ "(%s) I2C transfer failed: %d\n",
+ cmd_name, ret);
+ return ret;
+ }
- return 0;
+ if (ret != ARRAY_SIZE(msgs) ||
+ resp[FW_HDR_TYPE] != expected_response) {
+ if (--retries > 0) {
+ dev_dbg(&client->dev,
+ "(%s) unexpected response: %*ph (retrying)\n",
+ cmd_name, ret, resp);
+ continue;
+ }
+
+ dev_err(&client->dev,
+ "(%s) unexpected response: %*ph\n",
+ cmd_name, ret, resp);
+ return -EIO;
+ }
+
+ return --retries;
+ }
}
static int elants_i2c_calibrate(struct elants_data *ts)
@@ -313,27 +339,20 @@ static u16 elants_i2c_parse_version(u8 *buf)
static int elants_i2c_query_hw_version(struct elants_data *ts)
{
struct i2c_client *client = ts->client;
- int error, retry_cnt;
+ int retry_cnt = MAX_RETRIES;
const u8 cmd[] = { CMD_HEADER_READ, E_ELAN_INFO_FW_ID, 0x00, 0x01 };
u8 resp[HEADER_SIZE];
- for (retry_cnt = 0; retry_cnt < MAX_RETRIES; retry_cnt++) {
- error = elants_i2c_execute_command(client, cmd, sizeof(cmd),
- resp, sizeof(resp));
- if (!error) {
- ts->hw_version = elants_i2c_parse_version(resp);
- if (ts->hw_version != 0xffff)
- return 0;
- }
+ while (retry_cnt) {
+ retry_cnt = elants_i2c_execute_command(client, cmd, sizeof(cmd),
+ resp, sizeof(resp),
+ retry_cnt, "read fw id");
+ if (retry_cnt < 0)
+ return retry_cnt;
- dev_dbg(&client->dev, "read fw id error=%d, buf=%*phC\n",
- error, (int)sizeof(resp), resp);
- }
-
- if (error) {
- dev_err(&client->dev,
- "Failed to read fw id: %d\n", error);
- return error;
+ ts->hw_version = elants_i2c_parse_version(resp);
+ if (ts->hw_version != 0xffff)
+ return 0;
}
dev_err(&client->dev, "Invalid fw id: %#04x\n", ts->hw_version);
@@ -344,26 +363,28 @@ static int elants_i2c_query_hw_version(struct elants_data *ts)
static int elants_i2c_query_fw_version(struct elants_data *ts)
{
struct i2c_client *client = ts->client;
- int error, retry_cnt;
+ int retry_cnt = MAX_RETRIES;
const u8 cmd[] = { CMD_HEADER_READ, E_ELAN_INFO_FW_VER, 0x00, 0x01 };
u8 resp[HEADER_SIZE];
- for (retry_cnt = 0; retry_cnt < MAX_RETRIES; retry_cnt++) {
- error = elants_i2c_execute_command(client, cmd, sizeof(cmd),
- resp, sizeof(resp));
- if (!error) {
- ts->fw_version = elants_i2c_parse_version(resp);
- if (ts->fw_version != 0x0000 &&
- ts->fw_version != 0xffff)
- return 0;
- }
+ while (retry_cnt) {
+ retry_cnt = elants_i2c_execute_command(client, cmd,
+ sizeof(cmd),
+ resp, sizeof(resp),
+ retry_cnt,
+ "read fw version");
+ if (retry_cnt < 0)
+ return retry_cnt;
- dev_dbg(&client->dev, "read fw version error=%d, buf=%*phC\n",
- error, (int)sizeof(resp), resp);
+ ts->fw_version = elants_i2c_parse_version(resp);
+ if (ts->fw_version != 0x0000 && ts->fw_version != 0xffff)
+ return 0;
+
+ dev_dbg(&client->dev, "(read fw version) resp %*phC\n",
+ (int)sizeof(resp), resp);
}
- dev_err(&client->dev,
- "Failed to read fw version or fw version is invalid\n");
+ dev_err(&client->dev, "Invalid fw ver: %#04x\n", ts->fw_version);
return -EINVAL;
}
@@ -371,25 +392,20 @@ static int elants_i2c_query_fw_version(struct elants_data *ts)
static int elants_i2c_query_test_version(struct elants_data *ts)
{
struct i2c_client *client = ts->client;
- int error, retry_cnt;
+ int error;
u16 version;
const u8 cmd[] = { CMD_HEADER_READ, E_ELAN_INFO_TEST_VER, 0x00, 0x01 };
u8 resp[HEADER_SIZE];
- for (retry_cnt = 0; retry_cnt < MAX_RETRIES; retry_cnt++) {
- error = elants_i2c_execute_command(client, cmd, sizeof(cmd),
- resp, sizeof(resp));
- if (!error) {
- version = elants_i2c_parse_version(resp);
- ts->test_version = version >> 8;
- ts->solution_version = version & 0xff;
+ error = elants_i2c_execute_command(client, cmd, sizeof(cmd),
+ resp, sizeof(resp), MAX_RETRIES,
+ "read test version");
+ if (error >= 0) {
+ version = elants_i2c_parse_version(resp);
+ ts->test_version = version >> 8;
+ ts->solution_version = version & 0xff;
- return 0;
- }
-
- dev_dbg(&client->dev,
- "read test version error rc=%d, buf=%*phC\n",
- error, (int)sizeof(resp), resp);
+ return 0;
}
dev_err(&client->dev, "Failed to read test version\n");
@@ -406,13 +422,10 @@ static int elants_i2c_query_bc_version(struct elants_data *ts)
int error;
error = elants_i2c_execute_command(client, cmd, sizeof(cmd),
- resp, sizeof(resp));
- if (error) {
- dev_err(&client->dev,
- "read BC version error=%d, buf=%*phC\n",
- error, (int)sizeof(resp), resp);
+ resp, sizeof(resp), 1,
+ "read BC version");
+ if (error)
return error;
- }
version = elants_i2c_parse_version(resp);
ts->bc_version = version >> 8;
@@ -444,12 +457,10 @@ static int elants_i2c_query_ts_info(struct elants_data *ts)
error = elants_i2c_execute_command(client,
get_resolution_cmd,
sizeof(get_resolution_cmd),
- resp, sizeof(resp));
- if (error) {
- dev_err(&client->dev, "get resolution command failed: %d\n",
- error);
+ resp, sizeof(resp), 1,
+ "get resolution");
+ if (error)
return error;
- }
rows = resp[2] + resp[6] + resp[10];
cols = resp[3] + resp[7] + resp[11];
@@ -457,36 +468,29 @@ static int elants_i2c_query_ts_info(struct elants_data *ts)
/* Process mm_to_pixel information */
error = elants_i2c_execute_command(client,
get_osr_cmd, sizeof(get_osr_cmd),
- resp, sizeof(resp));
- if (error) {
- dev_err(&client->dev, "get osr command failed: %d\n",
- error);
+ resp, sizeof(resp), 1, "get osr");
+ if (error)
return error;
- }
osr = resp[3];
error = elants_i2c_execute_command(client,
get_physical_scan_cmd,
sizeof(get_physical_scan_cmd),
- resp, sizeof(resp));
- if (error) {
- dev_err(&client->dev, "get physical scan command failed: %d\n",
- error);
+ resp, sizeof(resp), 1,
+ "get physical scan");
+ if (error)
return error;
- }
phy_x = get_unaligned_be16(&resp[2]);
error = elants_i2c_execute_command(client,
get_physical_drive_cmd,
sizeof(get_physical_drive_cmd),
- resp, sizeof(resp));
- if (error) {
- dev_err(&client->dev, "get physical drive command failed: %d\n",
- error);
+ resp, sizeof(resp), 1,
+ "get physical drive");
+ if (error)
return error;
- }
phy_y = get_unaligned_be16(&resp[2]);
--
2.20.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 6/6] input: elants: read touchscreen size for EKTF3624
2019-12-10 0:19 [PATCH 0/6] input: elants: Support Asus TF300T touchscreen Michał Mirosław
` (2 preceding siblings ...)
2019-12-10 0:19 ` [PATCH 1/6] input: elants: document some registers and values Michał Mirosław
@ 2019-12-10 0:19 ` Michał Mirosław
2019-12-10 1:23 ` Dmitry Osipenko
2019-12-10 18:32 ` kbuild test robot
2019-12-10 0:19 ` [PATCH 5/6] input: elants: refactor elants_i2c_execute_command() Michał Mirosław
` (2 subsequent siblings)
6 siblings, 2 replies; 19+ messages in thread
From: Michał Mirosław @ 2019-12-10 0:19 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, linux-kernel, Dmitry Osipenko
EKTF3624 as present in Asus TF300T tablet has touchscreen size encoded
in different registers.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/input/touchscreen/elants_i2c.c | 77 ++++++++++++++++++++++++--
1 file changed, 72 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
index 27aca3971da5..e60a5eb9fb37 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -34,7 +34,7 @@
#include <linux/input/touchscreen.h>
#include <linux/input/mt.h>
#include <linux/acpi.h>
-#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/gpio/consumer.h>
#include <linux/regulator/consumer.h>
#include <asm/unaligned.h>
@@ -42,6 +42,10 @@
/* Device, Driver information */
#define DEVICE_NAME "elants_i2c"
+/* Device IDs */
+#define EKTH3500 0
+#define EKTF3624 1
+
/* Convert from rows or columns into resolution */
#define ELAN_TS_RESOLUTION(n, m) (((n) - 1) * (m))
@@ -160,6 +164,7 @@ struct elants_data {
bool wake_irq_enabled;
bool keep_power_in_suspend;
+ u8 chip_id;
/* Must be last to be used for DMA operations */
u8 buf[MAX_PACKET_SIZE] ____cacheline_aligned;
@@ -434,7 +439,53 @@ static int elants_i2c_query_bc_version(struct elants_data *ts)
return 0;
}
-static int elants_i2c_query_ts_info(struct elants_data *ts)
+static int elants_i2c_query_ts_info_ektf(struct elants_data *ts)
+{
+ struct i2c_client *client = ts->client;
+ int error;
+ u8 resp[4];
+ u16 phy_x, phy_y;
+ const u8 get_xres_cmd[] = {
+ CMD_HEADER_READ, E_INFO_X_RES, 0x00, 0x00
+ };
+ const u8 get_yres_cmd[] = {
+ CMD_HEADER_READ, E_INFO_Y_RES, 0x00, 0x00
+ };
+
+ /* Get X/Y size in mm */
+ error = elants_i2c_execute_command(client, get_xres_cmd,
+ sizeof(get_xres_cmd),
+ resp, sizeof(resp), 1,
+ "get X size");
+ if (error)
+ return error;
+
+ phy_x = resp[2] | ((resp[3] & 0xF0) << 4);
+
+ error = elants_i2c_execute_command(client, get_yres_cmd,
+ sizeof(get_yres_cmd),
+ resp, sizeof(resp), 1,
+ "get Y size");
+ if (error)
+ return error;
+
+ phy_y = resp[2] | ((resp[3] & 0xF0) << 4);
+
+ /* calculate resolution from size */
+ if (!ts->prop.max_x)
+ ts->prop.max_x = 2240;
+ ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
+
+ if (!ts->prop.max_y)
+ ts->prop.max_y = 1408;
+ ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
+
+ dev_dbg(&client->dev, "phy_x=%d, phy_y=%d\n", phy_x, phy_y);
+
+ return 0;
+}
+
+static int elants_i2c_query_ts_info_ekth(struct elants_data *ts)
{
struct i2c_client *client = ts->client;
int error;
@@ -587,8 +638,20 @@ static int elants_i2c_initialize(struct elants_data *ts)
error = elants_i2c_query_test_version(ts);
if (!error)
error = elants_i2c_query_bc_version(ts);
- if (!error)
- error = elants_i2c_query_ts_info(ts);
+
+ switch (ts->chip_id) {
+ case EKTH3500:
+ if (!error)
+ error = elants_i2c_query_ts_info_ekth(ts);
+ break;
+ case EKTF3624:
+ if (!error)
+ error = elants_i2c_query_ts_info_ektf(ts);
+ break;
+ default:
+ unreachable();
+ break;
+ }
if (error)
ts->iap_mode = ELAN_IAP_RECOVERY;
@@ -1185,6 +1248,9 @@ static int elants_i2c_probe(struct i2c_client *client,
ts->client = client;
i2c_set_clientdata(client, ts);
+ if (client->dev.of_node)
+ ts->chip_id = (uintptr_t)of_device_get_match_data(&client->dev);
+
ts->vcc33 = devm_regulator_get(&client->dev, "vcc33");
if (IS_ERR(ts->vcc33)) {
error = PTR_ERR(ts->vcc33);
@@ -1422,7 +1488,8 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id);
#ifdef CONFIG_OF
static const struct of_device_id elants_of_match[] = {
- { .compatible = "elan,ekth3500" },
+ { .compatible = "elan,ekth3500", .data = (void *)EKTH3500 },
+ { .compatible = "elan,ektf3624", .data = (void *)EKTF3624 },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, elants_of_match);
--
2.20.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/6] input: elants: detect max_x/y from hardware
2019-12-10 0:19 [PATCH 0/6] input: elants: Support Asus TF300T touchscreen Michał Mirosław
` (4 preceding siblings ...)
2019-12-10 0:19 ` [PATCH 5/6] input: elants: refactor elants_i2c_execute_command() Michał Mirosław
@ 2019-12-10 0:19 ` Michał Mirosław
2019-12-10 0:59 ` [PATCH 0/6] input: elants: Support Asus TF300T touchscreen Dmitry Osipenko
6 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2019-12-10 0:19 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, linux-kernel, Dmitry Osipenko
Read max_x/y from hardware when not specified in devicetree.
elants_i2c_initialize() call is moved after inputdev allocation (but
still before making it visible) to allow the function to see DT-provided
values.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/input/touchscreen/elants_i2c.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
index 02bd5e3e2171..2e6c9aa60496 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -498,9 +498,11 @@ static int elants_i2c_query_ts_info(struct elants_data *ts)
rows, cols, osr);
} else {
/* translate trace number to TS resolution */
- ts->prop.max_x = ELAN_TS_RESOLUTION(rows, osr);
+ if (!ts->prop.max_x)
+ ts->prop.max_x = ELAN_TS_RESOLUTION(rows, osr);
ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
- ts->prop.max_y = ELAN_TS_RESOLUTION(cols, osr);
+ if (!ts->prop.max_y)
+ ts->prop.max_y = ELAN_TS_RESOLUTION(cols, osr);
ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
}
@@ -1235,12 +1237,6 @@ static int elants_i2c_probe(struct i2c_client *client,
return -ENXIO;
}
- error = elants_i2c_initialize(ts);
- if (error) {
- dev_err(&client->dev, "failed to initialize: %d\n", error);
- return error;
- }
-
ts->input = devm_input_allocate_device(&client->dev);
if (!ts->input) {
dev_err(&client->dev, "Failed to allocate input device\n");
@@ -1252,6 +1248,12 @@ static int elants_i2c_probe(struct i2c_client *client,
touchscreen_parse_properties(ts->input, true, &ts->prop);
+ error = elants_i2c_initialize(ts);
+ if (error) {
+ dev_err(&client->dev, "failed to initialize: %d\n", error);
+ return error;
+ }
+
__set_bit(BTN_TOUCH, ts->input->keybit);
__set_bit(EV_ABS, ts->input->evbit);
__set_bit(EV_KEY, ts->input->evbit);
--
2.20.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 0/6] input: elants: Support Asus TF300T touchscreen
2019-12-10 0:19 [PATCH 0/6] input: elants: Support Asus TF300T touchscreen Michał Mirosław
` (5 preceding siblings ...)
2019-12-10 0:19 ` [PATCH 4/6] input: elants: detect max_x/y from hardware Michał Mirosław
@ 2019-12-10 0:59 ` Dmitry Osipenko
2019-12-10 15:26 ` Dmitry Osipenko
6 siblings, 1 reply; 19+ messages in thread
From: Dmitry Osipenko @ 2019-12-10 0:59 UTC (permalink / raw)
To: Michał Mirosław, linux-input; +Cc: Dmitry Torokhov, linux-kernel
10.12.2019 03:19, Michał Mirosław пишет:
> This series implements changes needed to support EKTF3624-based touchscreen
> used in eg. Asus Transformer Pad TF300T.
>
> Michał Mirosław (6):
> input: elants: document some registers and values
> input: elants: support old touch report format
> input: elants: support common touchscreen DT properties
> input: elants: detect max_x/y from hardware
> input: elants: refactor elants_i2c_execute_command()
> input: elants: read touchscreen size for EKTF3624
>
> drivers/input/touchscreen/elants_i2c.c | 365 ++++++++++++++++---------
> 1 file changed, 237 insertions(+), 128 deletions(-)
>
Hello Michał,
Very nice work! I saw these patches couple days ago on the rere git, but
wasn't sure if you were going to submit them. It's very good that you
found time to send them out, looks like we're going to have a fully
supported eKTF3624 in upstream soon!
It's a bit unfortunate that Elantech firmware writers couldn't maintain
common commands format for all firmware versions, but not really a big
deal since it will be one more trivial patch to support Nexus 7 :)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] input: elants: support common touchscreen DT properties
2019-12-10 0:19 ` [PATCH 3/6] input: elants: support common touchscreen DT properties Michał Mirosław
@ 2019-12-10 1:03 ` Dmitry Osipenko
2019-12-10 1:07 ` Dmitry Osipenko
2019-12-10 2:38 ` Michał Mirosław
0 siblings, 2 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2019-12-10 1:03 UTC (permalink / raw)
To: Michał Mirosław, linux-input; +Cc: Dmitry Torokhov, linux-kernel
10.12.2019 03:19, Michał Mirosław пишет:
> Support common DT properties like axis inversions to complement
> information obtained from device's firmware.
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> drivers/input/touchscreen/elants_i2c.c | 27 ++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
> index eadd26d5a06f..02bd5e3e2171 100644
> --- a/drivers/input/touchscreen/elants_i2c.c
> +++ b/drivers/input/touchscreen/elants_i2c.c
> @@ -31,6 +31,7 @@
> #include <linux/buffer_head.h>
> #include <linux/slab.h>
> #include <linux/firmware.h>
> +#include <linux/input/touchscreen.h>
> #include <linux/input/mt.h>
> #include <linux/acpi.h>
> #include <linux/of.h>
> @@ -146,8 +147,7 @@ struct elants_data {
> u16 hw_version;
> unsigned int x_res; /* resolution in units/mm */
> unsigned int y_res;
> - unsigned int x_max;
> - unsigned int y_max;
> + struct touchscreen_properties prop;
>
> enum elants_state state;
> enum elants_iap_mode iap_mode;
> @@ -498,10 +498,10 @@ static int elants_i2c_query_ts_info(struct elants_data *ts)
> rows, cols, osr);
> } else {
> /* translate trace number to TS resolution */
> - ts->x_max = ELAN_TS_RESOLUTION(rows, osr);
> - ts->x_res = DIV_ROUND_CLOSEST(ts->x_max, phy_x);
> - ts->y_max = ELAN_TS_RESOLUTION(cols, osr);
> - ts->y_res = DIV_ROUND_CLOSEST(ts->y_max, phy_y);
> + ts->prop.max_x = ELAN_TS_RESOLUTION(rows, osr);
> + ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
> + ts->prop.max_y = ELAN_TS_RESOLUTION(cols, osr);
> + ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
> }
>
> return 0;
> @@ -833,8 +833,7 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf,
>
> input_mt_slot(input, i);
> input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
> - input_event(input, EV_ABS, ABS_MT_POSITION_X, x);
> - input_event(input, EV_ABS, ABS_MT_POSITION_Y, y);
> + touchscreen_report_pos(input, &ts->prop, x, y, true);
> input_event(input, EV_ABS, ABS_MT_PRESSURE, p);
> input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, w);
>
> @@ -1251,13 +1250,15 @@ static int elants_i2c_probe(struct i2c_client *client,
> ts->input->name = "Elan Touchscreen";
> ts->input->id.bustype = BUS_I2C;
>
> + touchscreen_parse_properties(ts->input, true, &ts->prop);
Shouldn't this function be invoked after setting the max x/y sizes with
the hardware values? That's what all other drivers do and then you won't
need to set the ts->prop.max_x/y above in the code.
> __set_bit(BTN_TOUCH, ts->input->keybit);
> __set_bit(EV_ABS, ts->input->evbit);
> __set_bit(EV_KEY, ts->input->evbit);
>
> /* Single touch input params setup */
> - input_set_abs_params(ts->input, ABS_X, 0, ts->x_max, 0, 0);
> - input_set_abs_params(ts->input, ABS_Y, 0, ts->y_max, 0, 0);
> + input_set_abs_params(ts->input, ABS_X, 0, ts->prop.max_x, 0, 0);
> + input_set_abs_params(ts->input, ABS_Y, 0, ts->prop.max_y, 0, 0);
> input_set_abs_params(ts->input, ABS_PRESSURE, 0, 255, 0, 0);
> input_abs_set_res(ts->input, ABS_X, ts->x_res);
> input_abs_set_res(ts->input, ABS_Y, ts->y_res);
> @@ -1271,8 +1272,10 @@ static int elants_i2c_probe(struct i2c_client *client,
> return error;
> }
>
> - input_set_abs_params(ts->input, ABS_MT_POSITION_X, 0, ts->x_max, 0, 0);
> - input_set_abs_params(ts->input, ABS_MT_POSITION_Y, 0, ts->y_max, 0, 0);
> + input_set_abs_params(ts->input, ABS_MT_POSITION_X,
> + 0, ts->prop.max_x, 0, 0);
> + input_set_abs_params(ts->input, ABS_MT_POSITION_Y,
> + 0, ts->prop.max_y, 0, 0);
> input_set_abs_params(ts->input, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
> input_set_abs_params(ts->input, ABS_MT_PRESSURE, 0, 255, 0, 0);
> input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] input: elants: support common touchscreen DT properties
2019-12-10 1:03 ` Dmitry Osipenko
@ 2019-12-10 1:07 ` Dmitry Osipenko
2019-12-10 2:38 ` Michał Mirosław
1 sibling, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2019-12-10 1:07 UTC (permalink / raw)
To: Michał Mirosław, linux-input; +Cc: Dmitry Torokhov, linux-kernel
10.12.2019 04:03, Dmitry Osipenko пишет:
> 10.12.2019 03:19, Michał Mirosław пишет:
>> Support common DT properties like axis inversions to complement
>> information obtained from device's firmware.
>>
>> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>> ---
>> drivers/input/touchscreen/elants_i2c.c | 27 ++++++++++++++------------
>> 1 file changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
>> index eadd26d5a06f..02bd5e3e2171 100644
>> --- a/drivers/input/touchscreen/elants_i2c.c
>> +++ b/drivers/input/touchscreen/elants_i2c.c
>> @@ -31,6 +31,7 @@
>> #include <linux/buffer_head.h>
>> #include <linux/slab.h>
>> #include <linux/firmware.h>
>> +#include <linux/input/touchscreen.h>
>> #include <linux/input/mt.h>
>> #include <linux/acpi.h>
>> #include <linux/of.h>
>> @@ -146,8 +147,7 @@ struct elants_data {
>> u16 hw_version;
>> unsigned int x_res; /* resolution in units/mm */
>> unsigned int y_res;
>> - unsigned int x_max;
>> - unsigned int y_max;
>> + struct touchscreen_properties prop;
>>
>> enum elants_state state;
>> enum elants_iap_mode iap_mode;
>> @@ -498,10 +498,10 @@ static int elants_i2c_query_ts_info(struct elants_data *ts)
>> rows, cols, osr);
>> } else {
>> /* translate trace number to TS resolution */
>> - ts->x_max = ELAN_TS_RESOLUTION(rows, osr);
>> - ts->x_res = DIV_ROUND_CLOSEST(ts->x_max, phy_x);
>> - ts->y_max = ELAN_TS_RESOLUTION(cols, osr);
>> - ts->y_res = DIV_ROUND_CLOSEST(ts->y_max, phy_y);
>> + ts->prop.max_x = ELAN_TS_RESOLUTION(rows, osr);
>> + ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
>> + ts->prop.max_y = ELAN_TS_RESOLUTION(cols, osr);
>> + ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
>> }
>>
>> return 0;
>> @@ -833,8 +833,7 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf,
>>
>> input_mt_slot(input, i);
>> input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
>> - input_event(input, EV_ABS, ABS_MT_POSITION_X, x);
>> - input_event(input, EV_ABS, ABS_MT_POSITION_Y, y);
>> + touchscreen_report_pos(input, &ts->prop, x, y, true);
>> input_event(input, EV_ABS, ABS_MT_PRESSURE, p);
>> input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, w);
>>
>> @@ -1251,13 +1250,15 @@ static int elants_i2c_probe(struct i2c_client *client,
>> ts->input->name = "Elan Touchscreen";
>> ts->input->id.bustype = BUS_I2C;
>>
>> + touchscreen_parse_properties(ts->input, true, &ts->prop);
>
> Shouldn't this function be invoked after setting the max x/y sizes with
> the hardware values? That's what all other drivers do and then you won't
> need to set the ts->prop.max_x/y above in the code.
>> __set_bit(BTN_TOUCH, ts->input->keybit);
>> __set_bit(EV_ABS, ts->input->evbit);
>> __set_bit(EV_KEY, ts->input->evbit);
>>
>> /* Single touch input params setup */
>> - input_set_abs_params(ts->input, ABS_X, 0, ts->x_max, 0, 0);
>> - input_set_abs_params(ts->input, ABS_Y, 0, ts->y_max, 0, 0);
>> + input_set_abs_params(ts->input, ABS_X, 0, ts->prop.max_x, 0, 0);
>> + input_set_abs_params(ts->input, ABS_Y, 0, ts->prop.max_y, 0, 0);
>> input_set_abs_params(ts->input, ABS_PRESSURE, 0, 255, 0, 0);
>> input_abs_set_res(ts->input, ABS_X, ts->x_res);
>> input_abs_set_res(ts->input, ABS_Y, ts->y_res);
I'm actually wondering why this part is needed at all, the driver
doesn't report single-touch events.
>> @@ -1271,8 +1272,10 @@ static int elants_i2c_probe(struct i2c_client *client,
>> return error;
>> }
>>
>> - input_set_abs_params(ts->input, ABS_MT_POSITION_X, 0, ts->x_max, 0, 0);
>> - input_set_abs_params(ts->input, ABS_MT_POSITION_Y, 0, ts->y_max, 0, 0);
>> + input_set_abs_params(ts->input, ABS_MT_POSITION_X,
>> + 0, ts->prop.max_x, 0, 0);
>> + input_set_abs_params(ts->input, ABS_MT_POSITION_Y,
>> + 0, ts->prop.max_y, 0, 0);
>> input_set_abs_params(ts->input, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
>> input_set_abs_params(ts->input, ABS_MT_PRESSURE, 0, 255, 0, 0);
>> input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 6/6] input: elants: read touchscreen size for EKTF3624
2019-12-10 0:19 ` [PATCH 6/6] input: elants: read touchscreen size for EKTF3624 Michał Mirosław
@ 2019-12-10 1:23 ` Dmitry Osipenko
2019-12-10 1:48 ` Dmitry Osipenko
2019-12-10 18:32 ` kbuild test robot
1 sibling, 1 reply; 19+ messages in thread
From: Dmitry Osipenko @ 2019-12-10 1:23 UTC (permalink / raw)
To: Michał Mirosław, linux-input; +Cc: Dmitry Torokhov, linux-kernel
10.12.2019 03:19, Michał Mirosław пишет:
> EKTF3624 as present in Asus TF300T tablet has touchscreen size encoded
> in different registers.
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> drivers/input/touchscreen/elants_i2c.c | 77 ++++++++++++++++++++++++--
> 1 file changed, 72 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
> index 27aca3971da5..e60a5eb9fb37 100644
> --- a/drivers/input/touchscreen/elants_i2c.c
> +++ b/drivers/input/touchscreen/elants_i2c.c
> @@ -34,7 +34,7 @@
> #include <linux/input/touchscreen.h>
> #include <linux/input/mt.h>
> #include <linux/acpi.h>
> -#include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/gpio/consumer.h>
> #include <linux/regulator/consumer.h>
> #include <asm/unaligned.h>
> @@ -42,6 +42,10 @@
> /* Device, Driver information */
> #define DEVICE_NAME "elants_i2c"
>
> +/* Device IDs */
> +#define EKTH3500 0
> +#define EKTF3624 1
> +
> /* Convert from rows or columns into resolution */
> #define ELAN_TS_RESOLUTION(n, m) (((n) - 1) * (m))
>
> @@ -160,6 +164,7 @@ struct elants_data {
>
> bool wake_irq_enabled;
> bool keep_power_in_suspend;
> + u8 chip_id;
>
> /* Must be last to be used for DMA operations */
> u8 buf[MAX_PACKET_SIZE] ____cacheline_aligned;
> @@ -434,7 +439,53 @@ static int elants_i2c_query_bc_version(struct elants_data *ts)
> return 0;
> }
>
> -static int elants_i2c_query_ts_info(struct elants_data *ts)
> +static int elants_i2c_query_ts_info_ektf(struct elants_data *ts)
> +{
> + struct i2c_client *client = ts->client;
> + int error;
> + u8 resp[4];
> + u16 phy_x, phy_y;
> + const u8 get_xres_cmd[] = {
> + CMD_HEADER_READ, E_INFO_X_RES, 0x00, 0x00
> + };
> + const u8 get_yres_cmd[] = {
> + CMD_HEADER_READ, E_INFO_Y_RES, 0x00, 0x00
> + };
> +
> + /* Get X/Y size in mm */
> + error = elants_i2c_execute_command(client, get_xres_cmd,
> + sizeof(get_xres_cmd),
> + resp, sizeof(resp), 1,
> + "get X size");
> + if (error)
> + return error;
> +
> + phy_x = resp[2] | ((resp[3] & 0xF0) << 4);
> +
> + error = elants_i2c_execute_command(client, get_yres_cmd,
> + sizeof(get_yres_cmd),
> + resp, sizeof(resp), 1,
> + "get Y size");
> + if (error)
> + return error;
> +
> + phy_y = resp[2] | ((resp[3] & 0xF0) << 4);
> +
> + /* calculate resolution from size */
> + if (!ts->prop.max_x)
> + ts->prop.max_x = 2240;
> + ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
> +
> + if (!ts->prop.max_y)
> + ts->prop.max_y = 1408;
> + ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
> +
> + dev_dbg(&client->dev, "phy_x=%d, phy_y=%d\n", phy_x, phy_y);
> +
> + return 0;
> +}
> +
> +static int elants_i2c_query_ts_info_ekth(struct elants_data *ts)
> {
> struct i2c_client *client = ts->client;
> int error;
> @@ -587,8 +638,20 @@ static int elants_i2c_initialize(struct elants_data *ts)
> error = elants_i2c_query_test_version(ts);
> if (!error)
> error = elants_i2c_query_bc_version(ts);
> - if (!error)
> - error = elants_i2c_query_ts_info(ts);
> +
> + switch (ts->chip_id) {
> + case EKTH3500:
> + if (!error)
> + error = elants_i2c_query_ts_info_ekth(ts);
> + break;
> + case EKTF3624:
> + if (!error)
> + error = elants_i2c_query_ts_info_ektf(ts);
> + break;
> + default:
> + unreachable();
> + break;
> + }
>
> if (error)
> ts->iap_mode = ELAN_IAP_RECOVERY;
> @@ -1185,6 +1248,9 @@ static int elants_i2c_probe(struct i2c_client *client,
> ts->client = client;
> i2c_set_clientdata(client, ts);
>
> + if (client->dev.of_node)
> + ts->chip_id = (uintptr_t)of_device_get_match_data(&client->dev);
> +
> ts->vcc33 = devm_regulator_get(&client->dev, "vcc33");
> if (IS_ERR(ts->vcc33)) {
> error = PTR_ERR(ts->vcc33);
> @@ -1422,7 +1488,8 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id);
>
> #ifdef CONFIG_OF
> static const struct of_device_id elants_of_match[] = {
> - { .compatible = "elan,ekth3500" },
> + { .compatible = "elan,ekth3500", .data = (void *)EKTH3500 },
> + { .compatible = "elan,ektf3624", .data = (void *)EKTF3624 },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, elants_of_match);
It also should be possible to remove elants_of_match entirely and do the
following in the I2C ID table:
static const struct i2c_device_id elants_i2c_id[] = {
{ "ekth3500", EKTH3500 },
{ "ektf3624", EKTF3624 },
{ }
};
MODULE_DEVICE_TABLE(i2c, elants_i2c_id);
Then OF core will take care of device ID matching by removing the
"elan," part from the compatible value of the device-tree and comparing
it with the values in elants_i2c_id[].
And then in elants_i2c_probe() you could :
chip->chip_id = (uintptr_t)id->driver_data;
See "drivers/mfd/max77620.c" for the example.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/6] input: elants: refactor elants_i2c_execute_command()
2019-12-10 0:19 ` [PATCH 5/6] input: elants: refactor elants_i2c_execute_command() Michał Mirosław
@ 2019-12-10 1:34 ` Dmitry Osipenko
0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2019-12-10 1:34 UTC (permalink / raw)
To: Michał Mirosław, linux-input; +Cc: Dmitry Torokhov, linux-kernel
10.12.2019 03:19, Michał Mirosław пишет:
> Apply some DRY-ing to elants_i2c_execute_command() callers.
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> drivers/input/touchscreen/elants_i2c.c | 182 +++++++++++++------------
> 1 file changed, 93 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
> index 2e6c9aa60496..27aca3971da5 100644
> --- a/drivers/input/touchscreen/elants_i2c.c
> +++ b/drivers/input/touchscreen/elants_i2c.c
> @@ -201,7 +201,8 @@ static int elants_i2c_read(struct i2c_client *client, void *data, size_t size)
>
> static int elants_i2c_execute_command(struct i2c_client *client,
> const u8 *cmd, size_t cmd_size,
> - u8 *resp, size_t resp_size)
> + u8 *resp, size_t resp_size,
> + int retries, const char *cmd_name)
> {
> struct i2c_msg msgs[2];
> int ret;
> @@ -217,30 +218,55 @@ static int elants_i2c_execute_command(struct i2c_client *client,
> break;
>
> default:
> - dev_err(&client->dev, "%s: invalid command %*ph\n",
> - __func__, (int)cmd_size, cmd);
> + dev_err(&client->dev, "(%s): invalid command: %*ph\n",
> + cmd_name, (int)cmd_size, cmd);
> return -EINVAL;
> }
>
> - msgs[0].addr = client->addr;
> - msgs[0].flags = client->flags & I2C_M_TEN;
> - msgs[0].len = cmd_size;
> - msgs[0].buf = (u8 *)cmd;
> + for (;;) {
> + msgs[0].addr = client->addr;
> + msgs[0].flags = client->flags & I2C_M_TEN;
> + msgs[0].len = cmd_size;
> + msgs[0].buf = (u8 *)cmd;
>
> - msgs[1].addr = client->addr;
> - msgs[1].flags = client->flags & I2C_M_TEN;
> - msgs[1].flags |= I2C_M_RD;
> - msgs[1].len = resp_size;
> - msgs[1].buf = resp;
> + msgs[1].addr = client->addr;
> + msgs[1].flags = client->flags & I2C_M_TEN;
> + msgs[1].flags |= I2C_M_RD;
> + msgs[1].len = resp_size;
> + msgs[1].buf = resp;
>
> - ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> - if (ret < 0)
> - return ret;
> + ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> + if (ret < 0) {
> + if (--retries > 0) {
> + dev_dbg(&client->dev,
> + "(%s) I2C transfer failed: %d (retrying)\n",
> + cmd_name, ret);
> + continue;
> + }
>
> - if (ret != ARRAY_SIZE(msgs) || resp[FW_HDR_TYPE] != expected_response)
> - return -EIO;
> + dev_err(&client->dev,
> + "(%s) I2C transfer failed: %d\n",
> + cmd_name, ret);
> + return ret;
> + }
>
> - return 0;
> + if (ret != ARRAY_SIZE(msgs) ||
> + resp[FW_HDR_TYPE] != expected_response) {
> + if (--retries > 0) {
> + dev_dbg(&client->dev,
> + "(%s) unexpected response: %*ph (retrying)\n",
> + cmd_name, ret, resp);
> + continue;
> + }
> +
> + dev_err(&client->dev,
> + "(%s) unexpected response: %*ph\n",
> + cmd_name, ret, resp);
> + return -EIO;
> + }
> +
> + return --retries;
> + }
> }
>
> static int elants_i2c_calibrate(struct elants_data *ts)
> @@ -313,27 +339,20 @@ static u16 elants_i2c_parse_version(u8 *buf)
> static int elants_i2c_query_hw_version(struct elants_data *ts)
> {
> struct i2c_client *client = ts->client;
> - int error, retry_cnt;
> + int retry_cnt = MAX_RETRIES;
> const u8 cmd[] = { CMD_HEADER_READ, E_ELAN_INFO_FW_ID, 0x00, 0x01 };
> u8 resp[HEADER_SIZE];
>
> - for (retry_cnt = 0; retry_cnt < MAX_RETRIES; retry_cnt++) {
> - error = elants_i2c_execute_command(client, cmd, sizeof(cmd),
> - resp, sizeof(resp));
> - if (!error) {
> - ts->hw_version = elants_i2c_parse_version(resp);
> - if (ts->hw_version != 0xffff)
> - return 0;
> - }
> + while (retry_cnt) {
> + retry_cnt = elants_i2c_execute_command(client, cmd, sizeof(cmd),
> + resp, sizeof(resp),
> + retry_cnt, "read fw id");
> + if (retry_cnt < 0)
> + return retry_cnt;
>
> - dev_dbg(&client->dev, "read fw id error=%d, buf=%*phC\n",
> - error, (int)sizeof(resp), resp);
> - }
> -
> - if (error) {
> - dev_err(&client->dev,
> - "Failed to read fw id: %d\n", error);
> - return error;
> + ts->hw_version = elants_i2c_parse_version(resp);
> + if (ts->hw_version != 0xffff)
> + return 0;
> }
>
> dev_err(&client->dev, "Invalid fw id: %#04x\n", ts->hw_version);
> @@ -344,26 +363,28 @@ static int elants_i2c_query_hw_version(struct elants_data *ts)
> static int elants_i2c_query_fw_version(struct elants_data *ts)
> {
> struct i2c_client *client = ts->client;
> - int error, retry_cnt;
> + int retry_cnt = MAX_RETRIES;
> const u8 cmd[] = { CMD_HEADER_READ, E_ELAN_INFO_FW_VER, 0x00, 0x01 };
> u8 resp[HEADER_SIZE];
>
> - for (retry_cnt = 0; retry_cnt < MAX_RETRIES; retry_cnt++) {
> - error = elants_i2c_execute_command(client, cmd, sizeof(cmd),
> - resp, sizeof(resp));
> - if (!error) {
> - ts->fw_version = elants_i2c_parse_version(resp);
> - if (ts->fw_version != 0x0000 &&
> - ts->fw_version != 0xffff)
> - return 0;
> - }
> + while (retry_cnt) {
> + retry_cnt = elants_i2c_execute_command(client, cmd,
> + sizeof(cmd),
> + resp, sizeof(resp),
> + retry_cnt,
> + "read fw version");
> + if (retry_cnt < 0)
> + return retry_cnt;
>
> - dev_dbg(&client->dev, "read fw version error=%d, buf=%*phC\n",
> - error, (int)sizeof(resp), resp);
> + ts->fw_version = elants_i2c_parse_version(resp);
> + if (ts->fw_version != 0x0000 && ts->fw_version != 0xffff)
> + return 0;
> +
> + dev_dbg(&client->dev, "(read fw version) resp %*phC\n",
> + (int)sizeof(resp), resp);
> }
>
> - dev_err(&client->dev,
> - "Failed to read fw version or fw version is invalid\n");
> + dev_err(&client->dev, "Invalid fw ver: %#04x\n", ts->fw_version);
>
> return -EINVAL;
> }
> @@ -371,25 +392,20 @@ static int elants_i2c_query_fw_version(struct elants_data *ts)
> static int elants_i2c_query_test_version(struct elants_data *ts)
> {
> struct i2c_client *client = ts->client;
> - int error, retry_cnt;
> + int error;
> u16 version;
> const u8 cmd[] = { CMD_HEADER_READ, E_ELAN_INFO_TEST_VER, 0x00, 0x01 };
> u8 resp[HEADER_SIZE];
>
> - for (retry_cnt = 0; retry_cnt < MAX_RETRIES; retry_cnt++) {
> - error = elants_i2c_execute_command(client, cmd, sizeof(cmd),
> - resp, sizeof(resp));
> - if (!error) {
> - version = elants_i2c_parse_version(resp);
> - ts->test_version = version >> 8;
> - ts->solution_version = version & 0xff;
> + error = elants_i2c_execute_command(client, cmd, sizeof(cmd),
> + resp, sizeof(resp), MAX_RETRIES,
> + "read test version");
> + if (error >= 0) {
> + version = elants_i2c_parse_version(resp);
> + ts->test_version = version >> 8;
> + ts->solution_version = version & 0xff;
>
> - return 0;
> - }
> -
> - dev_dbg(&client->dev,
> - "read test version error rc=%d, buf=%*phC\n",
> - error, (int)sizeof(resp), resp);
> + return 0;
> }
>
> dev_err(&client->dev, "Failed to read test version\n");
> @@ -406,13 +422,10 @@ static int elants_i2c_query_bc_version(struct elants_data *ts)
> int error;
>
> error = elants_i2c_execute_command(client, cmd, sizeof(cmd),
> - resp, sizeof(resp));
> - if (error) {
> - dev_err(&client->dev,
> - "read BC version error=%d, buf=%*phC\n",
> - error, (int)sizeof(resp), resp);
> + resp, sizeof(resp), 1,
> + "read BC version");
> + if (error)
> return error;
> - }
>
> version = elants_i2c_parse_version(resp);
> ts->bc_version = version >> 8;
> @@ -444,12 +457,10 @@ static int elants_i2c_query_ts_info(struct elants_data *ts)
> error = elants_i2c_execute_command(client,
> get_resolution_cmd,
> sizeof(get_resolution_cmd),
> - resp, sizeof(resp));
> - if (error) {
> - dev_err(&client->dev, "get resolution command failed: %d\n",
> - error);
> + resp, sizeof(resp), 1,
> + "get resolution");
> + if (error)
> return error;
> - }
>
> rows = resp[2] + resp[6] + resp[10];
> cols = resp[3] + resp[7] + resp[11];
> @@ -457,36 +468,29 @@ static int elants_i2c_query_ts_info(struct elants_data *ts)
> /* Process mm_to_pixel information */
> error = elants_i2c_execute_command(client,
> get_osr_cmd, sizeof(get_osr_cmd),
> - resp, sizeof(resp));
> - if (error) {
> - dev_err(&client->dev, "get osr command failed: %d\n",
> - error);
> + resp, sizeof(resp), 1, "get osr");
> + if (error)
> return error;
> - }
>
> osr = resp[3];
>
> error = elants_i2c_execute_command(client,
> get_physical_scan_cmd,
> sizeof(get_physical_scan_cmd),
> - resp, sizeof(resp));
> - if (error) {
> - dev_err(&client->dev, "get physical scan command failed: %d\n",
> - error);
> + resp, sizeof(resp), 1,
> + "get physical scan");
> + if (error)
> return error;
> - }
>
> phy_x = get_unaligned_be16(&resp[2]);
>
> error = elants_i2c_execute_command(client,
> get_physical_drive_cmd,
> sizeof(get_physical_drive_cmd),
> - resp, sizeof(resp));
> - if (error) {
> - dev_err(&client->dev, "get physical drive command failed: %d\n",
> - error);
> + resp, sizeof(resp), 1,
> + "get physical drive");
> + if (error)
> return error;
> - }
>
> phy_y = get_unaligned_be16(&resp[2]);
>
>
Is this really needed? Could you please try to set IRQ_TYPE_EDGE_FALLING
in the device-tree for the interrupt type? I don't think think that
downstream kernel is correct and I've seen weird I2C problems when
IRQ_TYPE_LEVEL_LOW is used.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 6/6] input: elants: read touchscreen size for EKTF3624
2019-12-10 1:23 ` Dmitry Osipenko
@ 2019-12-10 1:48 ` Dmitry Osipenko
0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2019-12-10 1:48 UTC (permalink / raw)
To: Michał Mirosław, linux-input; +Cc: Dmitry Torokhov, linux-kernel
10.12.2019 04:23, Dmitry Osipenko пишет:
> 10.12.2019 03:19, Michał Mirosław пишет:
>> EKTF3624 as present in Asus TF300T tablet has touchscreen size encoded
>> in different registers.
>>
>> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>> ---
>> drivers/input/touchscreen/elants_i2c.c | 77 ++++++++++++++++++++++++--
>> 1 file changed, 72 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
>> index 27aca3971da5..e60a5eb9fb37 100644
>> --- a/drivers/input/touchscreen/elants_i2c.c
>> +++ b/drivers/input/touchscreen/elants_i2c.c
>> @@ -34,7 +34,7 @@
>> #include <linux/input/touchscreen.h>
>> #include <linux/input/mt.h>
>> #include <linux/acpi.h>
>> -#include <linux/of.h>
>> +#include <linux/of_device.h>
>> #include <linux/gpio/consumer.h>
>> #include <linux/regulator/consumer.h>
>> #include <asm/unaligned.h>
>> @@ -42,6 +42,10 @@
>> /* Device, Driver information */
>> #define DEVICE_NAME "elants_i2c"
>>
>> +/* Device IDs */
>> +#define EKTH3500 0
>> +#define EKTF3624 1
>> +
>> /* Convert from rows or columns into resolution */
>> #define ELAN_TS_RESOLUTION(n, m) (((n) - 1) * (m))
>>
>> @@ -160,6 +164,7 @@ struct elants_data {
>>
>> bool wake_irq_enabled;
>> bool keep_power_in_suspend;
>> + u8 chip_id;
>>
>> /* Must be last to be used for DMA operations */
>> u8 buf[MAX_PACKET_SIZE] ____cacheline_aligned;
>> @@ -434,7 +439,53 @@ static int elants_i2c_query_bc_version(struct elants_data *ts)
>> return 0;
>> }
>>
>> -static int elants_i2c_query_ts_info(struct elants_data *ts)
>> +static int elants_i2c_query_ts_info_ektf(struct elants_data *ts)
>> +{
>> + struct i2c_client *client = ts->client;
>> + int error;
>> + u8 resp[4];
>> + u16 phy_x, phy_y;
>> + const u8 get_xres_cmd[] = {
>> + CMD_HEADER_READ, E_INFO_X_RES, 0x00, 0x00
>> + };
>> + const u8 get_yres_cmd[] = {
>> + CMD_HEADER_READ, E_INFO_Y_RES, 0x00, 0x00
>> + };
>> +
>> + /* Get X/Y size in mm */
>> + error = elants_i2c_execute_command(client, get_xres_cmd,
>> + sizeof(get_xres_cmd),
>> + resp, sizeof(resp), 1,
>> + "get X size");
>> + if (error)
>> + return error;
>> +
>> + phy_x = resp[2] | ((resp[3] & 0xF0) << 4);
>> +
>> + error = elants_i2c_execute_command(client, get_yres_cmd,
>> + sizeof(get_yres_cmd),
>> + resp, sizeof(resp), 1,
>> + "get Y size");
>> + if (error)
>> + return error;
>> +
>> + phy_y = resp[2] | ((resp[3] & 0xF0) << 4);
>> +
>> + /* calculate resolution from size */
>> + if (!ts->prop.max_x)
>> + ts->prop.max_x = 2240;
>> + ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
>> +
>> + if (!ts->prop.max_y)
>> + ts->prop.max_y = 1408;
>> + ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
>> +
>> + dev_dbg(&client->dev, "phy_x=%d, phy_y=%d\n", phy_x, phy_y);
>> +
>> + return 0;
>> +}
>> +
>> +static int elants_i2c_query_ts_info_ekth(struct elants_data *ts)
>> {
>> struct i2c_client *client = ts->client;
>> int error;
>> @@ -587,8 +638,20 @@ static int elants_i2c_initialize(struct elants_data *ts)
>> error = elants_i2c_query_test_version(ts);
>> if (!error)
>> error = elants_i2c_query_bc_version(ts);
>> - if (!error)
>> - error = elants_i2c_query_ts_info(ts);
>> +
>> + switch (ts->chip_id) {
>> + case EKTH3500:
>> + if (!error)
>> + error = elants_i2c_query_ts_info_ekth(ts);
>> + break;
>> + case EKTF3624:
>> + if (!error)
>> + error = elants_i2c_query_ts_info_ektf(ts);
>> + break;
>> + default:
>> + unreachable();
>> + break;
>> + }
>>
>> if (error)
>> ts->iap_mode = ELAN_IAP_RECOVERY;
>> @@ -1185,6 +1248,9 @@ static int elants_i2c_probe(struct i2c_client *client,
>> ts->client = client;
>> i2c_set_clientdata(client, ts);
>>
>> + if (client->dev.of_node)
>> + ts->chip_id = (uintptr_t)of_device_get_match_data(&client->dev);
>> +
>> ts->vcc33 = devm_regulator_get(&client->dev, "vcc33");
>> if (IS_ERR(ts->vcc33)) {
>> error = PTR_ERR(ts->vcc33);
>> @@ -1422,7 +1488,8 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id);
>>
>> #ifdef CONFIG_OF
>> static const struct of_device_id elants_of_match[] = {
>> - { .compatible = "elan,ekth3500" },
>> + { .compatible = "elan,ekth3500", .data = (void *)EKTH3500 },
>> + { .compatible = "elan,ektf3624", .data = (void *)EKTF3624 },
>> { /* sentinel */ }
>> };
>> MODULE_DEVICE_TABLE(of, elants_of_match);
>
> It also should be possible to remove elants_of_match entirely and do the
> following in the I2C ID table:
>
> static const struct i2c_device_id elants_i2c_id[] = {
> { "ekth3500", EKTH3500 },
> { "ektf3624", EKTF3624 },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, elants_i2c_id);
>
> Then OF core will take care of device ID matching by removing the
> "elan," part from the compatible value of the device-tree and comparing
> it with the values in elants_i2c_id[].
>
> And then in elants_i2c_probe() you could :
>
> chip->chip_id = (uintptr_t)id->driver_data;
>
> See "drivers/mfd/max77620.c" for the example.
>
But maybe it's not really worth the effort at all since we need to
override the sizes in device-tree anyway?
What about to just ignore the firmware values of eKTF3624 for simplicity?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] input: elants: support common touchscreen DT properties
2019-12-10 1:03 ` Dmitry Osipenko
2019-12-10 1:07 ` Dmitry Osipenko
@ 2019-12-10 2:38 ` Michał Mirosław
2019-12-10 15:21 ` Dmitry Osipenko
1 sibling, 1 reply; 19+ messages in thread
From: Michał Mirosław @ 2019-12-10 2:38 UTC (permalink / raw)
To: Dmitry Osipenko; +Cc: linux-input, Dmitry Torokhov, linux-kernel
On Tue, Dec 10, 2019 at 04:03:18AM +0300, Dmitry Osipenko wrote:
> 10.12.2019 03:19, Michał Mirosław пишет:
> > Support common DT properties like axis inversions to complement
> > information obtained from device's firmware.a
[...]
> > @@ -498,10 +498,10 @@ static int elants_i2c_query_ts_info(struct elants_data *ts)
> > rows, cols, osr);
> > } else {
> > /* translate trace number to TS resolution */
> > - ts->x_max = ELAN_TS_RESOLUTION(rows, osr);
> > - ts->x_res = DIV_ROUND_CLOSEST(ts->x_max, phy_x);
> > - ts->y_max = ELAN_TS_RESOLUTION(cols, osr);
> > - ts->y_res = DIV_ROUND_CLOSEST(ts->y_max, phy_y);
> > + ts->prop.max_x = ELAN_TS_RESOLUTION(rows, osr);
> > + ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
> > + ts->prop.max_y = ELAN_TS_RESOLUTION(cols, osr);
> > + ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
> > }
> >
> > return 0;
> > @@ -833,8 +833,7 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf,
> >
> > input_mt_slot(input, i);
> > input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
> > - input_event(input, EV_ABS, ABS_MT_POSITION_X, x);
> > - input_event(input, EV_ABS, ABS_MT_POSITION_Y, y);
> > + touchscreen_report_pos(input, &ts->prop, x, y, true);
> > input_event(input, EV_ABS, ABS_MT_PRESSURE, p);
> > input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, w);
> >
> > @@ -1251,13 +1250,15 @@ static int elants_i2c_probe(struct i2c_client *client,
> > ts->input->name = "Elan Touchscreen";
> > ts->input->id.bustype = BUS_I2C;
> >
> > + touchscreen_parse_properties(ts->input, true, &ts->prop);
>
> Shouldn't this function be invoked after setting the max x/y sizes with
> the hardware values? That's what all other drivers do and then you won't
> need to set the ts->prop.max_x/y above in the code.
This is done later in the series - this patch only adds axis inversion
support and ignores DT-provided sizes.
Best Regards,
Michał Mirosław
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] input: elants: support common touchscreen DT properties
2019-12-10 2:38 ` Michał Mirosław
@ 2019-12-10 15:21 ` Dmitry Osipenko
2019-12-11 3:28 ` Michał Mirosław
0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Osipenko @ 2019-12-10 15:21 UTC (permalink / raw)
To: Michał Mirosław; +Cc: linux-input, Dmitry Torokhov, linux-kernel
10.12.2019 05:38, Michał Mirosław пишет:
> On Tue, Dec 10, 2019 at 04:03:18AM +0300, Dmitry Osipenko wrote:
>> 10.12.2019 03:19, Michał Mirosław пишет:
>>> Support common DT properties like axis inversions to complement
>>> information obtained from device's firmware.a
> [...]
>>> @@ -498,10 +498,10 @@ static int elants_i2c_query_ts_info(struct elants_data *ts)
>>> rows, cols, osr);
>>> } else {
>>> /* translate trace number to TS resolution */
>>> - ts->x_max = ELAN_TS_RESOLUTION(rows, osr);
>>> - ts->x_res = DIV_ROUND_CLOSEST(ts->x_max, phy_x);
>>> - ts->y_max = ELAN_TS_RESOLUTION(cols, osr);
>>> - ts->y_res = DIV_ROUND_CLOSEST(ts->y_max, phy_y);
>>> + ts->prop.max_x = ELAN_TS_RESOLUTION(rows, osr);
>>> + ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
>>> + ts->prop.max_y = ELAN_TS_RESOLUTION(cols, osr);
>>> + ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
>>> }
>>>
>>> return 0;
>>> @@ -833,8 +833,7 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf,
>>>
>>> input_mt_slot(input, i);
>>> input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
>>> - input_event(input, EV_ABS, ABS_MT_POSITION_X, x);
>>> - input_event(input, EV_ABS, ABS_MT_POSITION_Y, y);
>>> + touchscreen_report_pos(input, &ts->prop, x, y, true);
>>> input_event(input, EV_ABS, ABS_MT_PRESSURE, p);
>>> input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, w);
>>>
>>> @@ -1251,13 +1250,15 @@ static int elants_i2c_probe(struct i2c_client *client,
>>> ts->input->name = "Elan Touchscreen";
>>> ts->input->id.bustype = BUS_I2C;
>>>
>>> + touchscreen_parse_properties(ts->input, true, &ts->prop);
>>
>> Shouldn't this function be invoked after setting the max x/y sizes with
>> the hardware values? That's what all other drivers do and then you won't
>> need to set the ts->prop.max_x/y above in the code.
>
> This is done later in the series - this patch only adds axis inversion
> support and ignores DT-provided sizes.
What is the reason of splitting it into two patches?
Perhaps I'm still missing something, but why something a bit more simple
like this wouldn't yield exactly the same result:
diff --git a/drivers/input/touchscreen/elants_i2c.c
b/drivers/input/touchscreen/elants_i2c.c
index d4ad24ea54c8..8763a3b25c29 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -32,6 +32,7 @@
#include <linux/slab.h>
#include <linux/firmware.h>
#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
#include <linux/acpi.h>
#include <linux/of.h>
#include <linux/gpio/consumer.h>
@@ -147,6 +148,8 @@ struct elants_data {
/* Must be last to be used for DMA operations */
u8 buf[MAX_PACKET_SIZE] ____cacheline_aligned;
+
+ struct touchscreen_properties prop;
};
static int elants_i2c_send(struct i2c_client *client,
@@ -807,8 +810,7 @@ static void elants_i2c_mt_event(struct elants_data
*ts, u8 *buf)
input_mt_slot(input, i);
input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
- input_event(input, EV_ABS, ABS_MT_POSITION_X, x);
- input_event(input, EV_ABS, ABS_MT_POSITION_Y, y);
+ touchscreen_report_pos(ts->input, &ts->prop, x, y, true);
input_event(input, EV_ABS, ABS_MT_PRESSURE, p);
input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, w);
@@ -1249,6 +1251,8 @@ static int elants_i2c_probe(struct i2c_client *client,
input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
+ touchscreen_parse_properties(ts->input, true, &ts->prop);
+
error = input_register_device(ts->input);
if (error) {
dev_err(&client->dev,
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 0/6] input: elants: Support Asus TF300T touchscreen
2019-12-10 0:59 ` [PATCH 0/6] input: elants: Support Asus TF300T touchscreen Dmitry Osipenko
@ 2019-12-10 15:26 ` Dmitry Osipenko
0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2019-12-10 15:26 UTC (permalink / raw)
To: Michał Mirosław, linux-input; +Cc: Dmitry Torokhov, linux-kernel
10.12.2019 03:59, Dmitry Osipenko пишет:
> 10.12.2019 03:19, Michał Mirosław пишет:
>> This series implements changes needed to support EKTF3624-based touchscreen
>> used in eg. Asus Transformer Pad TF300T.
>>
>> Michał Mirosław (6):
>> input: elants: document some registers and values
>> input: elants: support old touch report format
>> input: elants: support common touchscreen DT properties
>> input: elants: detect max_x/y from hardware
>> input: elants: refactor elants_i2c_execute_command()
>> input: elants: read touchscreen size for EKTF3624
>>
>> drivers/input/touchscreen/elants_i2c.c | 365 ++++++++++++++++---------
>> 1 file changed, 237 insertions(+), 128 deletions(-)
>>
>
> Hello Michał,
>
> Very nice work! I saw these patches couple days ago on the rere git, but
> wasn't sure if you were going to submit them. It's very good that you
> found time to send them out, looks like we're going to have a fully
> supported eKTF3624 in upstream soon!
>
> It's a bit unfortunate that Elantech firmware writers couldn't maintain
> common commands format for all firmware versions, but not really a big
> deal since it will be one more trivial patch to support Nexus 7 :)
>
BTW, it could be worthwhile to CC Elan people on the next version of
this series:
Scott Liu <scott.liu@emc.com.tw>
James Chen <james.chen@emc.com.tw>
Johnny Chuang <johnny.chuang@emc.com.tw>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 6/6] input: elants: read touchscreen size for EKTF3624
2019-12-10 0:19 ` [PATCH 6/6] input: elants: read touchscreen size for EKTF3624 Michał Mirosław
2019-12-10 1:23 ` Dmitry Osipenko
@ 2019-12-10 18:32 ` kbuild test robot
1 sibling, 0 replies; 19+ messages in thread
From: kbuild test robot @ 2019-12-10 18:32 UTC (permalink / raw)
To: Michał Mirosław
Cc: kbuild-all, linux-input, Dmitry Torokhov, linux-kernel, Dmitry Osipenko
[-- Attachment #1: Type: text/plain, Size: 1214 bytes --]
Hi "Michał,
I love your patch! Perhaps something to improve:
[auto build test WARNING on input/next]
[also build test WARNING on v5.5-rc1 next-20191209]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Micha-Miros-aw/input-elants-document-some-registers-and-values/20191210-140810
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: x86_64-fedora-25 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-1) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/input/touchscreen/elants_i2c.o: warning: objtool: elants_i2c_initialize() falls through to next function elants_i2c_resume()
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51161 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] input: elants: support common touchscreen DT properties
2019-12-10 15:21 ` Dmitry Osipenko
@ 2019-12-11 3:28 ` Michał Mirosław
2019-12-11 15:26 ` Dmitry Osipenko
0 siblings, 1 reply; 19+ messages in thread
From: Michał Mirosław @ 2019-12-11 3:28 UTC (permalink / raw)
To: Dmitry Osipenko; +Cc: linux-input, Dmitry Torokhov, linux-kernel
On Tue, Dec 10, 2019 at 06:21:02PM +0300, Dmitry Osipenko wrote:
> 10.12.2019 05:38, Michał Mirosław пишет:
> > On Tue, Dec 10, 2019 at 04:03:18AM +0300, Dmitry Osipenko wrote:
> >> 10.12.2019 03:19, Michał Mirosław пишет:
> >>> Support common DT properties like axis inversions to complement
> >>> information obtained from device's firmware.a
> > [...]
> >>> @@ -1251,13 +1250,15 @@ static int elants_i2c_probe(struct i2c_client *client,
> >>> ts->input->name = "Elan Touchscreen";
> >>> ts->input->id.bustype = BUS_I2C;
> >>>
> >>> + touchscreen_parse_properties(ts->input, true, &ts->prop);
> >>
> >> Shouldn't this function be invoked after setting the max x/y sizes with
> >> the hardware values? That's what all other drivers do and then you won't
> >> need to set the ts->prop.max_x/y above in the code.
> >
> > This is done later in the series - this patch only adds axis inversion
> > support and ignores DT-provided sizes.
>
> What is the reason of splitting it into two patches?
>
> Perhaps I'm still missing something, but why something a bit more simple
> like this wouldn't yield exactly the same result:
[...]
Originally I thought to skip probing the hardware when all info is
already provided in devicetree. This didn't happen, though. I'll take
your patch then, with a slight adjustment in "prop"'s position... And
the rest of them, so as to not duplicate the work. :-)
Best Regards,
Michał Mirosław
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] input: elants: support common touchscreen DT properties
2019-12-11 3:28 ` Michał Mirosław
@ 2019-12-11 15:26 ` Dmitry Osipenko
0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2019-12-11 15:26 UTC (permalink / raw)
To: Michał Mirosław; +Cc: linux-input, Dmitry Torokhov, linux-kernel
11.12.2019 06:28, Michał Mirosław пишет:
> On Tue, Dec 10, 2019 at 06:21:02PM +0300, Dmitry Osipenko wrote:
>> 10.12.2019 05:38, Michał Mirosław пишет:
>>> On Tue, Dec 10, 2019 at 04:03:18AM +0300, Dmitry Osipenko wrote:
>>>> 10.12.2019 03:19, Michał Mirosław пишет:
>>>>> Support common DT properties like axis inversions to complement
>>>>> information obtained from device's firmware.a
>>> [...]
>>>>> @@ -1251,13 +1250,15 @@ static int elants_i2c_probe(struct i2c_client *client,
>>>>> ts->input->name = "Elan Touchscreen";
>>>>> ts->input->id.bustype = BUS_I2C;
>>>>>
>>>>> + touchscreen_parse_properties(ts->input, true, &ts->prop);
>>>>
>>>> Shouldn't this function be invoked after setting the max x/y sizes with
>>>> the hardware values? That's what all other drivers do and then you won't
>>>> need to set the ts->prop.max_x/y above in the code.
>>>
>>> This is done later in the series - this patch only adds axis inversion
>>> support and ignores DT-provided sizes.
>>
>> What is the reason of splitting it into two patches?
>>
>> Perhaps I'm still missing something, but why something a bit more simple
>> like this wouldn't yield exactly the same result:
> [...]
>
> Originally I thought to skip probing the hardware when all info is
> already provided in devicetree. This didn't happen, though. I'll take
> your patch then, with a slight adjustment in "prop"'s position... And
> the rest of them, so as to not duplicate the work. :-)
Okay
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-12-11 15:26 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10 0:19 [PATCH 0/6] input: elants: Support Asus TF300T touchscreen Michał Mirosław
2019-12-10 0:19 ` [PATCH 2/6] input: elants: support old touch report format Michał Mirosław
2019-12-10 0:19 ` [PATCH 3/6] input: elants: support common touchscreen DT properties Michał Mirosław
2019-12-10 1:03 ` Dmitry Osipenko
2019-12-10 1:07 ` Dmitry Osipenko
2019-12-10 2:38 ` Michał Mirosław
2019-12-10 15:21 ` Dmitry Osipenko
2019-12-11 3:28 ` Michał Mirosław
2019-12-11 15:26 ` Dmitry Osipenko
2019-12-10 0:19 ` [PATCH 1/6] input: elants: document some registers and values Michał Mirosław
2019-12-10 0:19 ` [PATCH 6/6] input: elants: read touchscreen size for EKTF3624 Michał Mirosław
2019-12-10 1:23 ` Dmitry Osipenko
2019-12-10 1:48 ` Dmitry Osipenko
2019-12-10 18:32 ` kbuild test robot
2019-12-10 0:19 ` [PATCH 5/6] input: elants: refactor elants_i2c_execute_command() Michał Mirosław
2019-12-10 1:34 ` Dmitry Osipenko
2019-12-10 0:19 ` [PATCH 4/6] input: elants: detect max_x/y from hardware Michał Mirosław
2019-12-10 0:59 ` [PATCH 0/6] input: elants: Support Asus TF300T touchscreen Dmitry Osipenko
2019-12-10 15:26 ` Dmitry Osipenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).