From: Nick Dyer <[email protected]>
The driver only registers one input device, which uses the screen
parameters from the first T9 instance. The first T63 instance also uses
those parameters.
It is incorrect to send input reports from the second instances of these
objects if they are enabled: the input scaling will be wrong and the
positions will be mashed together.
This also causes problems on Android if the number of slots exceeds 32.
In the future, this could be handled by looking for enabled touch object
instances and creating an input device for each one.
Signed-off-by: Nick Dyer <[email protected]>
Acked-by: Benson Leung <[email protected]>
Acked-by: Yufeng Shen <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 54fe190fd4bc..48c5ccab00a0 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1658,10 +1658,11 @@ static int mxt_parse_object_table(struct mxt_data *data,
break;
case MXT_TOUCH_MULTI_T9:
data->multitouch = MXT_TOUCH_MULTI_T9;
+ /* Only handle messages from first T9 instance */
data->T9_reportid_min = min_id;
- data->T9_reportid_max = max_id;
- data->num_touchids = object->num_report_ids
- * mxt_obj_instances(object);
+ data->T9_reportid_max = min_id +
+ object->num_report_ids - 1;
+ data->num_touchids = object->num_report_ids;
break;
case MXT_SPT_MESSAGECOUNT_T44:
data->T44_address = object->start_address;
--
2.17.1
From: Nick Dyer <[email protected]>
Signed-off-by: Nick Dyer <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 36 ++++++++++++------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 48c5ccab00a0..9555947a2d46 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -88,12 +88,12 @@
#define MXT_COMMAND_DIAGNOSTIC 5
/* Define for T6 status byte */
-#define MXT_T6_STATUS_RESET (1 << 7)
-#define MXT_T6_STATUS_OFL (1 << 6)
-#define MXT_T6_STATUS_SIGERR (1 << 5)
-#define MXT_T6_STATUS_CAL (1 << 4)
-#define MXT_T6_STATUS_CFGERR (1 << 3)
-#define MXT_T6_STATUS_COMSERR (1 << 2)
+#define MXT_T6_STATUS_RESET BIT(7)
+#define MXT_T6_STATUS_OFL BIT(6)
+#define MXT_T6_STATUS_SIGERR BIT(5)
+#define MXT_T6_STATUS_CAL BIT(4)
+#define MXT_T6_STATUS_CFGERR BIT(3)
+#define MXT_T6_STATUS_COMSERR BIT(2)
/* MXT_GEN_POWER_T7 field */
struct t7_config {
@@ -112,14 +112,14 @@ struct t7_config {
#define MXT_T9_RANGE 18
/* MXT_TOUCH_MULTI_T9 status */
-#define MXT_T9_UNGRIP (1 << 0)
-#define MXT_T9_SUPPRESS (1 << 1)
-#define MXT_T9_AMP (1 << 2)
-#define MXT_T9_VECTOR (1 << 3)
-#define MXT_T9_MOVE (1 << 4)
-#define MXT_T9_RELEASE (1 << 5)
-#define MXT_T9_PRESS (1 << 6)
-#define MXT_T9_DETECT (1 << 7)
+#define MXT_T9_UNGRIP BIT(0)
+#define MXT_T9_SUPPRESS BIT(1)
+#define MXT_T9_AMP BIT(2)
+#define MXT_T9_VECTOR BIT(3)
+#define MXT_T9_MOVE BIT(4)
+#define MXT_T9_RELEASE BIT(5)
+#define MXT_T9_PRESS BIT(6)
+#define MXT_T9_DETECT BIT(7)
struct t9_range {
__le16 x;
@@ -127,9 +127,9 @@ struct t9_range {
} __packed;
/* MXT_TOUCH_MULTI_T9 orient */
-#define MXT_T9_ORIENT_SWITCH (1 << 0)
-#define MXT_T9_ORIENT_INVERTX (1 << 1)
-#define MXT_T9_ORIENT_INVERTY (1 << 2)
+#define MXT_T9_ORIENT_SWITCH BIT(0)
+#define MXT_T9_ORIENT_INVERTX BIT(1)
+#define MXT_T9_ORIENT_INVERTY BIT(2)
/* MXT_SPT_COMMSCONFIG_T18 */
#define MXT_COMMS_CTRL 0
@@ -214,7 +214,7 @@ enum t100_type {
#define MXT_FRAME_CRC_PASS 0x04
#define MXT_APP_CRC_FAIL 0x40 /* valid 7 8 bit only */
#define MXT_BOOT_STATUS_MASK 0x3f
-#define MXT_BOOT_EXTENDED_ID (1 << 5)
+#define MXT_BOOT_EXTENDED_ID BIT(5)
#define MXT_BOOT_ID_MASK 0x1f
/* Touchscreen absolute values */
--
2.17.1
From: Nick Dyer <[email protected]>
Signed-off-by: Nick Dyer <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 9555947a2d46..dcafb812ee7e 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2055,12 +2055,6 @@ static int mxt_initialize_input_device(struct mxt_data *data)
0, 255, 0, 0);
}
- if (data->multitouch == MXT_TOUCH_MULTITOUCHSCREEN_T100 &&
- data->t100_aux_ampl) {
- input_set_abs_params(input_dev, ABS_MT_PRESSURE,
- 0, 255, 0, 0);
- }
-
if (data->multitouch == MXT_TOUCH_MULTITOUCHSCREEN_T100 &&
data->t100_aux_vect) {
input_set_abs_params(input_dev, ABS_MT_ORIENTATION,
--
2.17.1
From: Nick Dyer <[email protected]>
Signed-off-by: Nick Dyer <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index dcafb812ee7e..92661aa910ae 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1520,10 +1520,8 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
MXT_INFO_CHECKSUM_SIZE;
config_mem_size = data->mem_size - cfg_start_ofs;
config_mem = kzalloc(config_mem_size, GFP_KERNEL);
- if (!config_mem) {
- dev_err(dev, "Failed to allocate memory\n");
+ if (!config_mem)
return -ENOMEM;
- }
ret = mxt_prepare_cfg_mem(data, cfg, data_pos, cfg_start_ofs,
config_mem, config_mem_size);
@@ -1982,10 +1980,8 @@ static int mxt_initialize_input_device(struct mxt_data *data)
/* Register input device */
input_dev = input_allocate_device();
- if (!input_dev) {
- dev_err(dev, "Failed to allocate memory\n");
+ if (!input_dev)
return -ENOMEM;
- }
input_dev->name = "Atmel maXTouch Touchscreen";
input_dev->phys = data->phys;
--
2.17.1
From: Nick Dyer <[email protected]>
On devices with the T71 object, the config CRC will start there, rather
than at T7.
Signed-off-by: Nick Dyer <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 34 +++++++++++++++---------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 92661aa910ae..560d4997ef8c 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -75,6 +75,7 @@
#define MXT_SPT_DIGITIZER_T43 43
#define MXT_SPT_MESSAGECOUNT_T44 44
#define MXT_SPT_CTECONFIG_T46 46
+#define MXT_SPT_DYNAMICCONFIGURATIONCONTAINER_T71 71
#define MXT_TOUCH_MULTITOUCHSCREEN_T100 100
/* MXT_GEN_MESSAGE_T5 object */
@@ -317,6 +318,7 @@ struct mxt_data {
u8 T6_reportid;
u16 T6_address;
u16 T7_address;
+ u16 T71_address;
u8 T9_reportid_min;
u8 T9_reportid_max;
u8 T19_reportid;
@@ -382,6 +384,7 @@ static bool mxt_object_readable(unsigned int type)
case MXT_SPT_USERDATA_T38:
case MXT_SPT_DIGITIZER_T43:
case MXT_SPT_CTECONFIG_T46:
+ case MXT_SPT_DYNAMICCONFIGURATIONCONTAINER_T71:
return true;
default:
return false;
@@ -1443,6 +1446,7 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
u32 info_crc, config_crc, calculated_crc;
u8 *config_mem;
size_t config_mem_size;
+ u16 crc_start = 0;
mxt_update_crc(data, MXT_COMMAND_REPORTALL, 1);
@@ -1529,20 +1533,22 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
goto release_mem;
/* Calculate crc of the received configs (not the raw config file) */
- if (data->T7_address < cfg_start_ofs) {
- dev_err(dev, "Bad T7 address, T7addr = %x, config offset %x\n",
- data->T7_address, cfg_start_ofs);
- ret = 0;
- goto release_mem;
- }
+ if (data->T71_address)
+ crc_start = data->T71_address;
+ else if (data->T7_address)
+ crc_start = data->T7_address;
+ else
+ dev_warn(dev, "Could not find CRC start\n");
- calculated_crc = mxt_calculate_crc(config_mem,
- data->T7_address - cfg_start_ofs,
- config_mem_size);
+ if (crc_start > cfg_start_ofs) {
+ calculated_crc = mxt_calculate_crc(config_mem,
+ crc_start - cfg_start_ofs,
+ config_mem_size);
- if (config_crc > 0 && config_crc != calculated_crc)
- dev_warn(dev, "Config CRC error, calculated=%06X, file=%06X\n",
- calculated_crc, config_crc);
+ if (config_crc > 0 && config_crc != calculated_crc)
+ dev_warn(dev, "Config CRC in file inconsistent, calculated=%06X, file=%06X\n",
+ calculated_crc, config_crc);
+ }
ret = mxt_upload_cfg_mem(data, cfg_start_ofs,
config_mem, config_mem_size);
@@ -1589,6 +1595,7 @@ static void mxt_free_object_table(struct mxt_data *data)
data->T5_msg_size = 0;
data->T6_reportid = 0;
data->T7_address = 0;
+ data->T71_address = 0;
data->T9_reportid_min = 0;
data->T9_reportid_max = 0;
data->T19_reportid = 0;
@@ -1654,6 +1661,9 @@ static int mxt_parse_object_table(struct mxt_data *data,
case MXT_GEN_POWER_T7:
data->T7_address = object->start_address;
break;
+ case MXT_SPT_DYNAMICCONFIGURATIONCONTAINER_T71:
+ data->T71_address = object->start_address;
+ break;
case MXT_TOUCH_MULTI_T9:
data->multitouch = MXT_TOUCH_MULTI_T9;
/* Only handle messages from first T9 instance */
--
2.17.1
From: Nick Dyer <[email protected]>
input_mt_report_slot_state() ignores the tool when the slot is closed.
Remove the tool type from these function calls, which has caused a bit of
confusion.
Signed-off-by: Nick Dyer <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index d7023d261458..c31af790ef84 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -838,8 +838,7 @@ static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
* have happened.
*/
if (status & MXT_T9_RELEASE) {
- input_mt_report_slot_state(input_dev,
- MT_TOOL_FINGER, 0);
+ input_mt_report_slot_state(input_dev, 0, 0);
mxt_input_sync(data);
}
@@ -855,7 +854,7 @@ static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, area);
} else {
/* Touch no longer active, close out slot */
- input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, 0);
+ input_mt_report_slot_state(input_dev, 0, 0);
}
data->update_input = true;
--
2.17.1
From: Nick Dyer <[email protected]>
Signed-off-by: Nick Dyer <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index c31af790ef84..9f296a66c94e 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -728,13 +728,13 @@ static void mxt_proc_t6_messages(struct mxt_data *data, u8 *msg)
u8 status = msg[1];
u32 crc = msg[2] | (msg[3] << 8) | (msg[4] << 16);
- complete(&data->crc_completion);
-
if (crc != data->config_crc) {
data->config_crc = crc;
dev_dbg(dev, "T6 Config Checksum: 0x%06X\n", crc);
}
+ complete(&data->crc_completion);
+
/* Detect reset */
if (status & MXT_T6_STATUS_RESET)
complete(&data->reset_completion);
--
2.17.1
From: Nick Dyer <[email protected]>
We use sscanf to parse the configuration file, so it's necessary to zero
terminate the configuration otherwise a truncated file can cause the
parser to run off into uninitialised memory.
Signed-off-by: Nick Dyer <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 36 +++++++++++++++++-------
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 0ce126e918f1..2d1fddafb7f9 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -279,7 +279,7 @@ enum mxt_suspend_mode {
/* Config update context */
struct mxt_cfg {
- const u8 *raw;
+ u8 *raw;
size_t raw_size;
off_t raw_pos;
@@ -1451,14 +1451,21 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
u32 info_crc, config_crc, calculated_crc;
u16 crc_start = 0;
- cfg.raw = fw->data;
+ /* Make zero terminated copy of the OBP_RAW file */
+ cfg.raw = kzalloc(fw->size + 1, GFP_KERNEL);
+ if (!cfg.raw)
+ return -ENOMEM;
+
+ memcpy(cfg.raw, fw->data, fw->size);
+ cfg.raw[fw->size] = '\0';
cfg.raw_size = fw->size;
mxt_update_crc(data, MXT_COMMAND_REPORTALL, 1);
if (strncmp(cfg.raw, MXT_CFG_MAGIC, strlen(MXT_CFG_MAGIC))) {
dev_err(dev, "Unrecognised config file\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto release_raw;
}
cfg.raw_pos = strlen(MXT_CFG_MAGIC);
@@ -1470,7 +1477,8 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
&offset);
if (ret != 1) {
dev_err(dev, "Bad format\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto release_raw;
}
cfg.raw_pos += offset;
@@ -1478,26 +1486,30 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
if (cfg.info.family_id != data->info->family_id) {
dev_err(dev, "Family ID mismatch!\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto release_raw;
}
if (cfg.info.variant_id != data->info->variant_id) {
dev_err(dev, "Variant ID mismatch!\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto release_raw;
}
/* Read CRCs */
ret = sscanf(cfg.raw + cfg.raw_pos, "%x%n", &info_crc, &offset);
if (ret != 1) {
dev_err(dev, "Bad format: failed to parse Info CRC\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto release_raw;
}
cfg.raw_pos += offset;
ret = sscanf(cfg.raw + cfg.raw_pos, "%x%n", &config_crc, &offset);
if (ret != 1) {
dev_err(dev, "Bad format: failed to parse Config CRC\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto release_raw;
}
cfg.raw_pos += offset;
@@ -1530,8 +1542,10 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
MXT_INFO_CHECKSUM_SIZE;
cfg.mem_size = data->mem_size - cfg.start_ofs;
cfg.mem = kzalloc(cfg.mem_size, GFP_KERNEL);
- if (!cfg.mem)
- return -ENOMEM;
+ if (!cfg.mem) {
+ ret = -ENOMEM;
+ goto release_raw;
+ }
ret = mxt_prepare_cfg_mem(data, &cfg);
if (ret)
@@ -1570,6 +1584,8 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
/* T7 config may have changed */
mxt_init_t7_power_cfg(data);
+release_raw:
+ kfree(cfg.raw);
release_mem:
kfree(cfg.mem);
return ret;
--
2.17.1
From: Nick Dyer <[email protected]>
Signed-off-by: Nick Dyer <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 108 ++++++++++++-----------
1 file changed, 56 insertions(+), 52 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 560d4997ef8c..0ce126e918f1 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -277,6 +277,19 @@ enum mxt_suspend_mode {
MXT_SUSPEND_T9_CTRL = 1,
};
+/* Config update context */
+struct mxt_cfg {
+ const u8 *raw;
+ size_t raw_size;
+ off_t raw_pos;
+
+ u8 *mem;
+ size_t mem_size;
+ int start_ofs;
+
+ struct mxt_info info;
+};
+
/* Each client has this additional data */
struct mxt_data {
struct i2c_client *client;
@@ -1282,12 +1295,7 @@ static u32 mxt_calculate_crc(u8 *base, off_t start_off, off_t end_off)
return crc;
}
-static int mxt_prepare_cfg_mem(struct mxt_data *data,
- const struct firmware *cfg,
- unsigned int data_pos,
- unsigned int cfg_start_ofs,
- u8 *config_mem,
- size_t config_mem_size)
+static int mxt_prepare_cfg_mem(struct mxt_data *data, struct mxt_cfg *cfg)
{
struct device *dev = &data->client->dev;
struct mxt_object *object;
@@ -1298,9 +1306,9 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data,
u16 reg;
u8 val;
- while (data_pos < cfg->size) {
+ while (cfg->raw_pos < cfg->raw_size) {
/* Read type, instance, length */
- ret = sscanf(cfg->data + data_pos, "%x %x %x%n",
+ ret = sscanf(cfg->raw + cfg->raw_pos, "%x %x %x%n",
&type, &instance, &size, &offset);
if (ret == 0) {
/* EOF */
@@ -1309,20 +1317,20 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data,
dev_err(dev, "Bad format: failed to parse object\n");
return -EINVAL;
}
- data_pos += offset;
+ cfg->raw_pos += offset;
object = mxt_get_object(data, type);
if (!object) {
/* Skip object */
for (i = 0; i < size; i++) {
- ret = sscanf(cfg->data + data_pos, "%hhx%n",
+ ret = sscanf(cfg->raw + cfg->raw_pos, "%hhx%n",
&val, &offset);
if (ret != 1) {
dev_err(dev, "Bad format in T%d at %d\n",
type, i);
return -EINVAL;
}
- data_pos += offset;
+ cfg->raw_pos += offset;
}
continue;
}
@@ -1357,7 +1365,7 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data,
reg = object->start_address + mxt_obj_size(object) * instance;
for (i = 0; i < size; i++) {
- ret = sscanf(cfg->data + data_pos, "%hhx%n",
+ ret = sscanf(cfg->raw + cfg->raw_pos, "%hhx%n",
&val,
&offset);
if (ret != 1) {
@@ -1365,15 +1373,15 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data,
type, i);
return -EINVAL;
}
- data_pos += offset;
+ cfg->raw_pos += offset;
if (i > mxt_obj_size(object))
continue;
- byte_offset = reg + i - cfg_start_ofs;
+ byte_offset = reg + i - cfg->start_ofs;
- if (byte_offset >= 0 && byte_offset < config_mem_size) {
- *(config_mem + byte_offset) = val;
+ if (byte_offset >= 0 && byte_offset < cfg->mem_size) {
+ *(cfg->mem + byte_offset) = val;
} else {
dev_err(dev, "Bad object: reg:%d, T%d, ofs=%d\n",
reg, object->type, byte_offset);
@@ -1385,22 +1393,21 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data,
return 0;
}
-static int mxt_upload_cfg_mem(struct mxt_data *data, unsigned int cfg_start,
- u8 *config_mem, size_t config_mem_size)
+static int mxt_upload_cfg_mem(struct mxt_data *data, struct mxt_cfg *cfg)
{
unsigned int byte_offset = 0;
int error;
/* Write configuration as blocks */
- while (byte_offset < config_mem_size) {
- unsigned int size = config_mem_size - byte_offset;
+ while (byte_offset < cfg->mem_size) {
+ unsigned int size = cfg->mem_size - byte_offset;
if (size > MXT_MAX_BLOCK_WRITE)
size = MXT_MAX_BLOCK_WRITE;
error = __mxt_write_reg(data->client,
- cfg_start + byte_offset,
- size, config_mem + byte_offset);
+ cfg->start_ofs + byte_offset,
+ size, cfg->mem + byte_offset);
if (error) {
dev_err(&data->client->dev,
"Config write error, ret=%d\n", error);
@@ -1434,66 +1441,65 @@ static int mxt_init_t7_power_cfg(struct mxt_data *data);
* <SIZE> - 2-byte object size as hex
* <CONTENTS> - array of <SIZE> 1-byte hex values
*/
-static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
+static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
{
struct device *dev = &data->client->dev;
- struct mxt_info cfg_info;
+ struct mxt_cfg cfg;
int ret;
int offset;
- int data_pos;
int i;
- int cfg_start_ofs;
u32 info_crc, config_crc, calculated_crc;
- u8 *config_mem;
- size_t config_mem_size;
u16 crc_start = 0;
+ cfg.raw = fw->data;
+ cfg.raw_size = fw->size;
+
mxt_update_crc(data, MXT_COMMAND_REPORTALL, 1);
- if (strncmp(cfg->data, MXT_CFG_MAGIC, strlen(MXT_CFG_MAGIC))) {
+ if (strncmp(cfg.raw, MXT_CFG_MAGIC, strlen(MXT_CFG_MAGIC))) {
dev_err(dev, "Unrecognised config file\n");
return -EINVAL;
}
- data_pos = strlen(MXT_CFG_MAGIC);
+ cfg.raw_pos = strlen(MXT_CFG_MAGIC);
/* Load information block and check */
for (i = 0; i < sizeof(struct mxt_info); i++) {
- ret = sscanf(cfg->data + data_pos, "%hhx%n",
- (unsigned char *)&cfg_info + i,
+ ret = sscanf(cfg.raw + cfg.raw_pos, "%hhx%n",
+ (unsigned char *)&cfg.info + i,
&offset);
if (ret != 1) {
dev_err(dev, "Bad format\n");
return -EINVAL;
}
- data_pos += offset;
+ cfg.raw_pos += offset;
}
- if (cfg_info.family_id != data->info->family_id) {
+ if (cfg.info.family_id != data->info->family_id) {
dev_err(dev, "Family ID mismatch!\n");
return -EINVAL;
}
- if (cfg_info.variant_id != data->info->variant_id) {
+ if (cfg.info.variant_id != data->info->variant_id) {
dev_err(dev, "Variant ID mismatch!\n");
return -EINVAL;
}
/* Read CRCs */
- ret = sscanf(cfg->data + data_pos, "%x%n", &info_crc, &offset);
+ ret = sscanf(cfg.raw + cfg.raw_pos, "%x%n", &info_crc, &offset);
if (ret != 1) {
dev_err(dev, "Bad format: failed to parse Info CRC\n");
return -EINVAL;
}
- data_pos += offset;
+ cfg.raw_pos += offset;
- ret = sscanf(cfg->data + data_pos, "%x%n", &config_crc, &offset);
+ ret = sscanf(cfg.raw + cfg.raw_pos, "%x%n", &config_crc, &offset);
if (ret != 1) {
dev_err(dev, "Bad format: failed to parse Config CRC\n");
return -EINVAL;
}
- data_pos += offset;
+ cfg.raw_pos += offset;
/*
* The Info Block CRC is calculated over mxt_info and the object
@@ -1519,16 +1525,15 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
}
/* Malloc memory to store configuration */
- cfg_start_ofs = MXT_OBJECT_START +
+ cfg.start_ofs = MXT_OBJECT_START +
data->info->object_num * sizeof(struct mxt_object) +
MXT_INFO_CHECKSUM_SIZE;
- config_mem_size = data->mem_size - cfg_start_ofs;
- config_mem = kzalloc(config_mem_size, GFP_KERNEL);
- if (!config_mem)
+ cfg.mem_size = data->mem_size - cfg.start_ofs;
+ cfg.mem = kzalloc(cfg.mem_size, GFP_KERNEL);
+ if (!cfg.mem)
return -ENOMEM;
- ret = mxt_prepare_cfg_mem(data, cfg, data_pos, cfg_start_ofs,
- config_mem, config_mem_size);
+ ret = mxt_prepare_cfg_mem(data, &cfg);
if (ret)
goto release_mem;
@@ -1540,18 +1545,17 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
else
dev_warn(dev, "Could not find CRC start\n");
- if (crc_start > cfg_start_ofs) {
- calculated_crc = mxt_calculate_crc(config_mem,
- crc_start - cfg_start_ofs,
- config_mem_size);
+ if (crc_start > cfg.start_ofs) {
+ calculated_crc = mxt_calculate_crc(cfg.mem,
+ crc_start - cfg.start_ofs,
+ cfg.mem_size);
if (config_crc > 0 && config_crc != calculated_crc)
dev_warn(dev, "Config CRC in file inconsistent, calculated=%06X, file=%06X\n",
calculated_crc, config_crc);
}
- ret = mxt_upload_cfg_mem(data, cfg_start_ofs,
- config_mem, config_mem_size);
+ ret = mxt_upload_cfg_mem(data, &cfg);
if (ret)
goto release_mem;
@@ -1567,7 +1571,7 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
mxt_init_t7_power_cfg(data);
release_mem:
- kfree(config_mem);
+ kfree(cfg.mem);
return ret;
}
--
2.17.1
From: Nick Dyer <[email protected]>
If T9.CTRL DISAMP is set, then pressure is reported as zero. This means
some app layers (eg tslib) will ignore the contact.
Signed-off-by: Nick Dyer <[email protected]>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 2d1fddafb7f9..d7023d261458 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -843,6 +843,10 @@ static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
mxt_input_sync(data);
}
+ /* if active, pressure must be non-zero */
+ if (!amplitude)
+ amplitude = MXT_PRESSURE_DEFAULT;
+
/* Touch active */
input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, 1);
input_report_abs(input_dev, ABS_MT_POSITION_X, x);
--
2.17.1
On Fri, Jul 20, 2018 at 10:51:21PM +0100, Nick Dyer wrote:
> From: Nick Dyer <[email protected]>
>
> input_mt_report_slot_state() ignores the tool when the slot is closed.
> Remove the tool type from these function calls, which has caused a bit of
> confusion.
Hmm, maybe we could introduce MT_TOOL_NONE or MT_TOOL_INACTIVE and get
rid of the 3rd parameter? It will require a bit of macro trickery for a
release or 2...
>
> Signed-off-by: Nick Dyer <[email protected]>
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index d7023d261458..c31af790ef84 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -838,8 +838,7 @@ static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
> * have happened.
> */
> if (status & MXT_T9_RELEASE) {
> - input_mt_report_slot_state(input_dev,
> - MT_TOOL_FINGER, 0);
> + input_mt_report_slot_state(input_dev, 0, 0);
> mxt_input_sync(data);
> }
>
> @@ -855,7 +854,7 @@ static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
> input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, area);
> } else {
> /* Touch no longer active, close out slot */
> - input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, 0);
> + input_mt_report_slot_state(input_dev, 0, 0);
> }
>
> data->update_input = true;
> --
> 2.17.1
>
--
Dmitry
On Fri, Jul 20, 2018 at 10:51:19PM +0100, Nick Dyer wrote:
> From: Nick Dyer <[email protected]>
>
> We use sscanf to parse the configuration file, so it's necessary to zero
> terminate the configuration otherwise a truncated file can cause the
> parser to run off into uninitialised memory.
>
> Signed-off-by: Nick Dyer <[email protected]>
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 36 +++++++++++++++++-------
> 1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 0ce126e918f1..2d1fddafb7f9 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -279,7 +279,7 @@ enum mxt_suspend_mode {
>
> /* Config update context */
> struct mxt_cfg {
> - const u8 *raw;
> + u8 *raw;
> size_t raw_size;
> off_t raw_pos;
>
> @@ -1451,14 +1451,21 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
> u32 info_crc, config_crc, calculated_crc;
> u16 crc_start = 0;
>
> - cfg.raw = fw->data;
> + /* Make zero terminated copy of the OBP_RAW file */
> + cfg.raw = kzalloc(fw->size + 1, GFP_KERNEL);
kmemdup_nul()? I guess config it not that big to be concerned with
kmalloc() vs vmalloc() and allocation failures...
> + if (!cfg.raw)
> + return -ENOMEM;
> +
> + memcpy(cfg.raw, fw->data, fw->size);
> + cfg.raw[fw->size] = '\0';
> cfg.raw_size = fw->size;
>
> mxt_update_crc(data, MXT_COMMAND_REPORTALL, 1);
>
> if (strncmp(cfg.raw, MXT_CFG_MAGIC, strlen(MXT_CFG_MAGIC))) {
> dev_err(dev, "Unrecognised config file\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto release_raw;
> }
>
> cfg.raw_pos = strlen(MXT_CFG_MAGIC);
> @@ -1470,7 +1477,8 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
> &offset);
> if (ret != 1) {
> dev_err(dev, "Bad format\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto release_raw;
> }
>
> cfg.raw_pos += offset;
> @@ -1478,26 +1486,30 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
>
> if (cfg.info.family_id != data->info->family_id) {
> dev_err(dev, "Family ID mismatch!\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto release_raw;
> }
>
> if (cfg.info.variant_id != data->info->variant_id) {
> dev_err(dev, "Variant ID mismatch!\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto release_raw;
> }
>
> /* Read CRCs */
> ret = sscanf(cfg.raw + cfg.raw_pos, "%x%n", &info_crc, &offset);
> if (ret != 1) {
> dev_err(dev, "Bad format: failed to parse Info CRC\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto release_raw;
> }
> cfg.raw_pos += offset;
>
> ret = sscanf(cfg.raw + cfg.raw_pos, "%x%n", &config_crc, &offset);
> if (ret != 1) {
> dev_err(dev, "Bad format: failed to parse Config CRC\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto release_raw;
> }
> cfg.raw_pos += offset;
>
> @@ -1530,8 +1542,10 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
> MXT_INFO_CHECKSUM_SIZE;
> cfg.mem_size = data->mem_size - cfg.start_ofs;
> cfg.mem = kzalloc(cfg.mem_size, GFP_KERNEL);
> - if (!cfg.mem)
> - return -ENOMEM;
> + if (!cfg.mem) {
> + ret = -ENOMEM;
> + goto release_raw;
> + }
>
> ret = mxt_prepare_cfg_mem(data, &cfg);
> if (ret)
> @@ -1570,6 +1584,8 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
> /* T7 config may have changed */
> mxt_init_t7_power_cfg(data);
>
> +release_raw:
> + kfree(cfg.raw);
> release_mem:
> kfree(cfg.mem);
> return ret;
> --
> 2.17.1
>
--
Dmitry
On Tue, Jul 24, 2018 at 12:34 AM Dmitry Torokhov
<[email protected]> wrote:
>
> On Fri, Jul 20, 2018 at 10:51:21PM +0100, Nick Dyer wrote:
> > From: Nick Dyer <[email protected]>
> >
> > input_mt_report_slot_state() ignores the tool when the slot is closed.
> > Remove the tool type from these function calls, which has caused a bit of
> > confusion.
>
> Hmm, maybe we could introduce MT_TOOL_NONE or MT_TOOL_INACTIVE and get
> rid of the 3rd parameter? It will require a bit of macro trickery for a
> release or 2...
I am not sure what would be the benefit of adding those new tools, if
the input_mt code discards them. Do you want to forward them to the
userspace with the release?
This reminds me the discussion we had recently with the touchscreens
releasing the slots with a MT_TOOL_PALM.
Anyway, better include Peter as he will be using this new MT_TOOL.
Cheers,
Benjamin
>
> >
> > Signed-off-by: Nick Dyer <[email protected]>
> > ---
> > drivers/input/touchscreen/atmel_mxt_ts.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> > index d7023d261458..c31af790ef84 100644
> > --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> > @@ -838,8 +838,7 @@ static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
> > * have happened.
> > */
> > if (status & MXT_T9_RELEASE) {
> > - input_mt_report_slot_state(input_dev,
> > - MT_TOOL_FINGER, 0);
> > + input_mt_report_slot_state(input_dev, 0, 0);
> > mxt_input_sync(data);
> > }
> >
> > @@ -855,7 +854,7 @@ static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
> > input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, area);
> > } else {
> > /* Touch no longer active, close out slot */
> > - input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, 0);
> > + input_mt_report_slot_state(input_dev, 0, 0);
> > }
> >
> > data->update_input = true;
> > --
> > 2.17.1
> >
>
> --
> Dmitry
On Mon, Jul 23, 2018 at 03:35:34PM -0700, Dmitry Torokhov wrote:
> On Fri, Jul 20, 2018 at 10:51:19PM +0100, Nick Dyer wrote:
> > From: Nick Dyer <[email protected]>
> >
> > We use sscanf to parse the configuration file, so it's necessary to zero
> > terminate the configuration otherwise a truncated file can cause the
> > parser to run off into uninitialised memory.
> >
> > Signed-off-by: Nick Dyer <[email protected]>
> > ---
> > drivers/input/touchscreen/atmel_mxt_ts.c | 36 +++++++++++++++++-------
> > 1 file changed, 26 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> > index 0ce126e918f1..2d1fddafb7f9 100644
> > --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> > @@ -279,7 +279,7 @@ enum mxt_suspend_mode {
> >
> > /* Config update context */
> > struct mxt_cfg {
> > - const u8 *raw;
> > + u8 *raw;
> > size_t raw_size;
> > off_t raw_pos;
> >
> > @@ -1451,14 +1451,21 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
> > u32 info_crc, config_crc, calculated_crc;
> > u16 crc_start = 0;
> >
> > - cfg.raw = fw->data;
> > + /* Make zero terminated copy of the OBP_RAW file */
> > + cfg.raw = kzalloc(fw->size + 1, GFP_KERNEL);
>
> kmemdup_nul()? I guess config it not that big to be concerned with
> kmalloc() vs vmalloc() and allocation failures...
Yes, that looks like it should work. There's a limit on the size of the
data due to the I2C address space, so we should be fine.
Nick
On Tue, Jul 24, 2018 at 10:23:27AM +0200, Benjamin Tissoires wrote:
> On Tue, Jul 24, 2018 at 12:34 AM Dmitry Torokhov
> <[email protected]> wrote:
> >
> > On Fri, Jul 20, 2018 at 10:51:21PM +0100, Nick Dyer wrote:
> > > From: Nick Dyer <[email protected]>
> > >
> > > input_mt_report_slot_state() ignores the tool when the slot is closed.
> > > Remove the tool type from these function calls, which has caused a bit of
> > > confusion.
> >
> > Hmm, maybe we could introduce MT_TOOL_NONE or MT_TOOL_INACTIVE and get
> > rid of the 3rd parameter? It will require a bit of macro trickery for a
> > release or 2...
>
> I am not sure what would be the benefit of adding those new tools, if
> the input_mt code discards them. Do you want to forward them to the
> userspace with the release?
> This reminds me the discussion we had recently with the touchscreens
> releasing the slots with a MT_TOOL_PALM.
>
> Anyway, better include Peter as he will be using this new MT_TOOL.
thanks for the CC, would've missed this.
From what I read this would be a helper for internal changes only, not
exposed to userspace? If so maybe it's better/easier/more readable to break
it into two functions
input_mt_open_slot(dev, MT_TOOL_FINGER)
input_mt_close_slot(dev)
This removes any ambiguity about the handling of the tool and should be a
fairly trivial search/replace. Replace the 'open/close' terminology with
whatever suits better.
Cheers,
Peter
> > > Signed-off-by: Nick Dyer <[email protected]>
> > > ---
> > > drivers/input/touchscreen/atmel_mxt_ts.c | 5 ++---
> > > 1 file changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> > > index d7023d261458..c31af790ef84 100644
> > > --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> > > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> > > @@ -838,8 +838,7 @@ static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
> > > * have happened.
> > > */
> > > if (status & MXT_T9_RELEASE) {
> > > - input_mt_report_slot_state(input_dev,
> > > - MT_TOOL_FINGER, 0);
> > > + input_mt_report_slot_state(input_dev, 0, 0);
> > > mxt_input_sync(data);
> > > }
> > >
> > > @@ -855,7 +854,7 @@ static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
> > > input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, area);
> > > } else {
> > > /* Touch no longer active, close out slot */
> > > - input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, 0);
> > > + input_mt_report_slot_state(input_dev, 0, 0);
> > > }
> > >
> > > data->update_input = true;
> > > --
> > > 2.17.1
> > >
> >
> > --
> > Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Wed, Jul 25, 2018 at 03:26:41PM +1000, Peter Hutterer wrote:
> On Tue, Jul 24, 2018 at 10:23:27AM +0200, Benjamin Tissoires wrote:
> > On Tue, Jul 24, 2018 at 12:34 AM Dmitry Torokhov
> > <[email protected]> wrote:
> > >
> > > On Fri, Jul 20, 2018 at 10:51:21PM +0100, Nick Dyer wrote:
> > > > From: Nick Dyer <[email protected]>
> > > >
> > > > input_mt_report_slot_state() ignores the tool when the slot is closed.
> > > > Remove the tool type from these function calls, which has caused a bit of
> > > > confusion.
> > >
> > > Hmm, maybe we could introduce MT_TOOL_NONE or MT_TOOL_INACTIVE and get
> > > rid of the 3rd parameter? It will require a bit of macro trickery for a
> > > release or 2...
> >
> > I am not sure what would be the benefit of adding those new tools, if
> > the input_mt code discards them. Do you want to forward them to the
> > userspace with the release?
> > This reminds me the discussion we had recently with the touchscreens
> > releasing the slots with a MT_TOOL_PALM.
> >
> > Anyway, better include Peter as he will be using this new MT_TOOL.
>
> thanks for the CC, would've missed this.
>
> From what I read this would be a helper for internal changes only, not
> exposed to userspace? If so maybe it's better/easier/more readable to break
> it into two functions
> input_mt_open_slot(dev, MT_TOOL_FINGER)
> input_mt_close_slot(dev)
>
> This removes any ambiguity about the handling of the tool and should be a
> fairly trivial search/replace. Replace the 'open/close' terminology with
> whatever suits better.
Hmm, I do like the "input_mt_close_slot()", or
"input_mt_report_slot_inactive()". I think the
input_mt_report_slot_state() is fine for "opening" the slot, as, with it
now returning bool, we can do:
if (input_mt_report_slot_state(dev, MT_TOOL_FINGER, state)) {
...
< report events for active slot >
}
Thanks.
--
Dmitry
On Fri, Jul 20, 2018 at 10:51:13PM +0100, Nick Dyer wrote:
> From: Nick Dyer <[email protected]>
>
> The driver only registers one input device, which uses the screen
> parameters from the first T9 instance. The first T63 instance also uses
> those parameters.
>
> It is incorrect to send input reports from the second instances of these
> objects if they are enabled: the input scaling will be wrong and the
> positions will be mashed together.
>
> This also causes problems on Android if the number of slots exceeds 32.
>
> In the future, this could be handled by looking for enabled touch object
> instances and creating an input device for each one.
>
> Signed-off-by: Nick Dyer <[email protected]>
> Acked-by: Benson Leung <[email protected]>
> Acked-by: Yufeng Shen <[email protected]>
> ---
OK, I adjusted patch #7 to use kmemdup_nul() as we discussed, and
skipped #9, applied the rest.
Thanks!
> drivers/input/touchscreen/atmel_mxt_ts.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 54fe190fd4bc..48c5ccab00a0 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -1658,10 +1658,11 @@ static int mxt_parse_object_table(struct mxt_data *data,
> break;
> case MXT_TOUCH_MULTI_T9:
> data->multitouch = MXT_TOUCH_MULTI_T9;
> + /* Only handle messages from first T9 instance */
> data->T9_reportid_min = min_id;
> - data->T9_reportid_max = max_id;
> - data->num_touchids = object->num_report_ids
> - * mxt_obj_instances(object);
> + data->T9_reportid_max = min_id +
> + object->num_report_ids - 1;
> + data->num_touchids = object->num_report_ids;
> break;
> case MXT_SPT_MESSAGECOUNT_T44:
> data->T44_address = object->start_address;
> --
> 2.17.1
>
--
Dmitry