Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v1 59/63] Input: atmel_mxt_ts: Prevent crash due to freeing of input device
From: Jiada Wang @ 2019-08-16  8:38 UTC (permalink / raw)
  To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190816083830.19553-1-jiada_wang@mentor.com>

From: Bhuvanesh Surachari <Bhuvanesh_Surachari@mentor.com>

Move sysfs creation after intializing the input device to
prevent crash due to freeing of input device by function via sysfs.

Signed-off-by: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com>
Signed-off-by: Sanjeev Chugh <sanjeev_chugh@mentor.com>
Signed-off-by: George G. Davis <george_davis@mentor.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 431c2c54eab0..8e95f46a30d7 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2962,6 +2962,8 @@ static int mxt_initialize_input_device(struct mxt_data *data)
 		goto err_free_mem;
 	}
 
+	data->input_dev = input_dev;
+
 	if (data->gpio_attrs.attrs) {
 		error = sysfs_create_group(&input_dev->dev.kobj,
 					   &data->gpio_attrs);
@@ -2972,11 +2974,10 @@ static int mxt_initialize_input_device(struct mxt_data *data)
 		}
 	}
 
-	data->input_dev = input_dev;
-
 	return 0;
 
 err_free_mem:
+	data->input_dev = NULL;
 	input_free_device(input_dev);
 	return error;
 }
@@ -4597,13 +4598,6 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		msleep(MXT_RESET_TIME);
 	}
 
-	error = sysfs_create_group(&client->dev.kobj, &mxt_fw_attr_group);
-	if (error) {
-		dev_err(&client->dev, "Failure %d creating fw sysfs group\n",
-			error);
-		return error;
-	}
-
 	INIT_WORK(&data->watchdog_work, mxt_watchdog_work);
 
 	/* setup watchdog timer */
@@ -4613,11 +4607,18 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 	error = mxt_initialize(data);
 	if (error)
-		goto err_free_object;
+		goto err_del_wd_timer;
+
+	error = sysfs_create_group(&client->dev.kobj, &mxt_fw_attr_group);
+	if (error) {
+		dev_err(&client->dev, "Failure %d creating fw sysfs group\n",
+			error);
+		goto err_del_wd_timer;
+	}
 
 	return 0;
 
-err_free_object:
+err_del_wd_timer:
 	cancel_work_sync(&data->watchdog_work);
 	mxt_stop_wd_timer(data);
 	mxt_free_input_device(data);
-- 
2.19.2

^ permalink raw reply related

* [PATCH v1 60/63] input: atmel_mxt_ts: added sysfs interface to update atmel T38 data
From: Jiada Wang @ 2019-08-16  8:38 UTC (permalink / raw)
  To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis

From: Naveen Chakka <Naveen.Chakka@in.bosch.com>

Atmel touch controller contains T38 object where a user can store its own
data of length 64 bytes. T38 data will not be part of checksum
calculation on executing T6 BACKUP command.

format used to update the T38 data is given below:

<offset> <length> <actual_data>

offset: offset address of the data to be written in the t38 object
	(in decimal)

length: length of the data to be written into the t38 object(in decimal)

data: actual data bytes to be written into the t38 object
      (values should be in hex)

Ex:
1. 0 2 10 20
updates first two bytes of the t38 data with values 10 and 20

2. 19 6 10 2f 30 4a 50 60
updates 6 bytes of t38 data from the index 19-24 with hex values

Signed-off-by: Naveen Chakka <Naveen.Chakka@in.bosch.com>
Signed-off-by: Sanjeev Chugh <sanjeev_chugh@mentor.com>
Signed-off-by: George G. Davis <george_davis@mentor.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 104 ++++++++++++++++++++++-
 1 file changed, 102 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 8e95f46a30d7..fd9a96ec3fd3 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -3024,8 +3024,6 @@ static void mxt_watchdog_timer(struct timer_list *t)
 {
 	struct mxt_data *data = from_timer(data, t, watchdog_timer);
 
-	dev_dbg(&data->client->dev, "%s: Timer triggered\n", __func__);
-
 	if (!work_pending(&data->watchdog_work)) {
 		if (!data->mxt_status.intp_triggered)
 			schedule_work(&data->watchdog_work);
@@ -4095,6 +4093,106 @@ static ssize_t mxt_touch_device_status(struct device *dev, struct
 	return ret;
 }
 
+static ssize_t mxt_t38_data_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct mxt_data *data = dev_get_drvdata(dev);
+	struct mxt_object *object;
+	size_t count = 0, size;
+	u8 i, *t38_buf;
+
+	if (!data->object_table)
+		return -ENXIO;
+
+	object = mxt_get_object(data, MXT_SPT_USERDATA_T38);
+	size = mxt_obj_size(object);
+
+	/* Pre-allocate buffer large enough to hold max size of t38 object.*/
+	t38_buf = kmalloc(size, GFP_KERNEL);
+	if (!t38_buf)
+		return -ENOMEM;
+
+	count = __mxt_read_reg(data->client, object->start_address,
+			       size, t38_buf);
+	if (count)
+		goto end;
+
+	for (i = 0; i < size; i++)
+		count += scnprintf(buf + count, PAGE_SIZE - count,
+				   "[%2u]: %02x\n", i, t38_buf[i]);
+	count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
+end:
+	kfree(t38_buf);
+	return count;
+}
+
+static ssize_t mxt_t38_data_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct mxt_data *data = dev_get_drvdata(dev);
+	struct mxt_object *object;
+	ssize_t ret = 0, pos, offset;
+	unsigned int i, len, index;
+	u8 *t38_buf;
+
+	if (!data->object_table)
+		return -ENXIO;
+
+	object = mxt_get_object(data, MXT_SPT_USERDATA_T38);
+
+	/* Pre-allocate buffer large enough to hold max size of t38 object.*/
+	t38_buf = kmalloc(mxt_obj_size(object), GFP_KERNEL);
+	if (!t38_buf)
+		return -ENOMEM;
+
+	ret = sscanf(buf, "%zd %d%zd", &offset, &len, &pos);
+	if (ret != 2) {
+		dev_err(dev, "Bad format: Invalid parameter to update t38\n");
+		ret = -EINVAL;
+		goto end;
+	}
+
+	if (len == 0) {
+		dev_err(dev,
+			"Bad format: Data length should not be equal to 0\n");
+		ret = -EINVAL;
+		goto end;
+	}
+
+	if (offset < 0 || ((offset + len) > 64)) {
+		dev_err(dev, "Invalid offset value to update t38\n");
+		ret = -EINVAL;
+		goto end;
+	}
+
+	index = pos;
+	for (i = 0; i < len; i++) {
+		ret = sscanf(buf + index, "%hhx%zd", t38_buf + i, &pos);
+		if (ret != 1) {
+			dev_err(dev, "Bad format: Invalid Data\n");
+			ret = -EINVAL;
+			goto end;
+		}
+		index += pos;
+	}
+
+	ret = __mxt_write_reg(data->client, object->start_address + offset,
+			      len, t38_buf);
+	if (ret)
+		goto end;
+
+	ret = mxt_t6_command(data, MXT_COMMAND_BACKUPNV, MXT_BACKUP_VALUE,
+			     true);
+	if (ret)
+		dev_err(dev, "backup command failed\n");
+	else
+		ret = count;
+end:
+	kfree(t38_buf);
+	return ret;
+}
+
 static DEVICE_ATTR(fw_version, S_IRUGO, mxt_fw_version_show, NULL);
 static DEVICE_ATTR(hw_version, S_IRUGO, mxt_hw_version_show, NULL);
 static DEVICE_ATTR(object, S_IRUGO, mxt_object_show, NULL);
@@ -4107,6 +4205,7 @@ static DEVICE_ATTR(debug_v2_enable, S_IWUSR | S_IRUSR, NULL,
 static DEVICE_ATTR(debug_notify, S_IRUGO, mxt_debug_notify_show, NULL);
 static DEVICE_ATTR(t25, 0600, mxt_t25_selftest_show, mxt_t25_selftest_store);
 static DEVICE_ATTR(touch_dev_stat, 0444, mxt_touch_device_status, NULL);
+static DEVICE_ATTR(t38_data, 0600, mxt_t38_data_show, mxt_t38_data_store);
 
 static struct attribute *mxt_attrs[] = {
 	&dev_attr_fw_version.attr,
@@ -4119,6 +4218,7 @@ static struct attribute *mxt_attrs[] = {
 	&dev_attr_debug_notify.attr,
 	&dev_attr_t25.attr,
 	&dev_attr_touch_dev_stat.attr,
+	&dev_attr_t38_data.attr,
 	NULL
 };
 
-- 
2.19.2

^ permalink raw reply related

* [PATCH v1 61/63] input: atmel_mxt_ts: Add NULL check for sysfs attribute debug_msg_attr
From: Jiada Wang @ 2019-08-16  8:39 UTC (permalink / raw)
  To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190816083902.19659-1-jiada_wang@mentor.com>

From: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>

In some scenarios mxt_debug_msg_remove() can be called from
mxt_remove() even before the data->debug_msg_attr is updated.
So a NULL check is required for data->debug_msg_attr before
accessing it.
There is also an additional change in mxt_debug_msg_init() to update
data->debug_msg_attr only when sysfs_create_bin_file is successful.

Consider the following scenario: Atmel MXT driver module is being loaded
hence mxt_probe() is called, then mxt_probe() calls mxt_initialize() and
in mxt_initialize() function if the Atmel MXT is in bootloader mode there
is a possibility that it can return before calling mxt_debug_msg_init(),
with a value of 0 meaning no error.
In this case data->debug_msg_attr is now at the initialized state only,
which is NULL as allocation of data pointer was done with devm_kzalloc()
call in mxt_probe function. Completion of initialisation will be deferred
until some later time.
If there is now an attempt to remove the Atmel MXT driver module, in
mxt_remove() there is a call to mxt_debug_msg_remove() which calls
sysfs_remove_bin_file() with a NULL data->debug_msg_attr.
Therefore, we need to have check in mxt_debug_msg_remove() for
data->debug_msg_attr being NULL.

Signed-off-by: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
Signed-off-by: Mark Craske <Mark_Craske@mentor.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index fd9a96ec3fd3..1179f90a8077 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -615,21 +615,26 @@ static int mxt_debug_msg_init(struct mxt_data *data)
 	 * so it is safe to update a single struct bin_attribute entity
 	 */
 	debug_msg_attr.size = data->T5_msg_size * DEBUG_MSG_MAX;
-	data->debug_msg_attr = &debug_msg_attr;
 
 	if (sysfs_create_bin_file(&data->client->dev.kobj,
-				  data->debug_msg_attr) < 0) {
+				  &debug_msg_attr) < 0) {
 		dev_err(&data->client->dev, "Failed to create %s\n",
 			debug_msg_attr.attr.name);
 		return -EINVAL;
 	}
 
+	data->debug_msg_attr = &debug_msg_attr;
+
 	return 0;
 }
 
 static void mxt_debug_msg_remove(struct mxt_data *data)
 {
-	sysfs_remove_bin_file(&data->client->dev.kobj, data->debug_msg_attr);
+	if (data->debug_msg_attr) {
+		sysfs_remove_bin_file(&data->client->dev.kobj,
+				      data->debug_msg_attr);
+		data->debug_msg_attr = NULL;
+	}
 }
 
 static int mxt_wait_for_completion(struct mxt_data *data,
-- 
2.19.2

^ permalink raw reply related

* [PATCH v1 62/63] Input: atmel_mxt_ts: Implement synchronization during various operation
From: Jiada Wang @ 2019-08-16  8:39 UTC (permalink / raw)
  To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190816083902.19659-1-jiada_wang@mentor.com>

From: Sanjeev Chugh <sanjeev_chugh@mentor.com>

There could be scope of race conditions when sysfs is being handled
and at the same time, device removal is occurring. For example,
we don't want the device removal to begin if the Atmel device
cfg update is going on or firmware update is going on. In such
cases, wait for device update to be completed before the removal
continues.

    Thread                                          Thread 2:
=========================                       =========================
mxt_update_fw_store()                           mxt_remove()
mutex_lock(&data->lock)                         ...
mxt_initialize()                                //Tries to acquire lock
  request_firmware_nowait()                     mutex_lock(&data->lock)
...                                             ==>waits for lock()
...                                             .
...                                             .
mutex_unlock(&data->lock)                       .
                                                //Gets lock and proceeds
                                                mxt_free_input_device();
                                                ...
                                                mutex_unlock(&data->lock)
                                                //Frees atmel driver data
                                                kfree(data)

If the request_firmware_nowait() completes after the driver removal,
and callback is triggered. But kernel crashes since the module is
already removed.

This commit adds state machine to serialize such scenarios.

Signed-off-by: Sanjeev Chugh <sanjeev_chugh@mentor.com>
Signed-off-by: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 222 ++++++++++++++++++++---
 1 file changed, 196 insertions(+), 26 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 1179f90a8077..6e5fad91c379 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -224,6 +224,7 @@ enum t100_type {
 #define MXT_POWERON_DELAY	150	/* msec */
 #define MXT_BOOTLOADER_WAIT	36E5	/* 1 minute */
 #define MXT_WATCHDOG_TIMEOUT	1000	/* msec */
+#define MXT_CONFIG_TIMEOUT	1000	/* msec */
 
 /* Command to unlock bootloader */
 #define MXT_UNLOCK_CMD_MSB	0xaa
@@ -247,6 +248,20 @@ enum t100_type {
 
 #define DEBUG_MSG_MAX		200
 
+enum device_state {
+	MXT_STATE_READY,
+	MXT_STATE_UPDATING_CONFIG,
+	MXT_STATE_UPDATING_CONFIG_ASYNC,
+	MXT_STATE_START,
+	MXT_STATE_STOP,
+	MXT_STATE_GOING_AWAY
+};
+
+enum mxt_cmd {
+	UPDATE_CFG,
+	UPDATE_FW
+};
+
 struct mxt_info {
 	u8 family_id;
 	u8 variant_id;
@@ -427,8 +442,7 @@ struct mxt_data {
 	/* Indicates whether device is in suspend */
 	bool suspended;
 
-	/* Indicates whether device is updating configuration */
-	bool updating_config;
+	struct mutex lock;
 
 	unsigned long gpio_input_pin_status;
 	struct attribute_group gpio_attrs;
@@ -436,6 +450,11 @@ struct mxt_data {
 	unsigned int mtu;
 
 	bool t25_status;
+
+	/* State handling for probe/remove, open/close and config update */
+	enum device_state e_state;
+
+	struct completion update_cfg_completion;
 };
 
 struct mxt_gpio_attr {
@@ -1664,6 +1683,7 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)
 	struct mxt_data *data = dev_id;
 	int ret;
 
+	mutex_lock(&data->lock);
 	data->mxt_status.intp_triggered = true;
 
 	if (data->in_bootloader) {
@@ -1691,6 +1711,8 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)
 
 exit:
 	data->mxt_status.intp_triggered = false;
+	mutex_unlock(&data->lock);
+
 	return ret;
 }
 
@@ -2275,6 +2297,8 @@ static void mxt_free_object_table(struct mxt_data *data)
 	video_unregister_device(&data->dbg.vdev);
 	v4l2_device_unregister(&data->dbg.v4l2);
 #endif
+	mutex_lock(&data->lock);
+
 	data->object_table = NULL;
 	kfree(data->info);
 	data->info = NULL;
@@ -2304,6 +2328,8 @@ static void mxt_free_object_table(struct mxt_data *data)
 	data->T100_reportid_min = 0;
 	data->T100_reportid_max = 0;
 	data->max_reportid = 0;
+
+	mutex_unlock(&data->lock);
 }
 
 static int mxt_parse_object_table(struct mxt_data *data,
@@ -2995,8 +3021,15 @@ static int mxt_configure_objects(struct mxt_data *data,
 
 static void mxt_config_cb(const struct firmware *cfg, void *ctx)
 {
+	struct mxt_data *data = ctx;
+
 	mxt_configure_objects(ctx, cfg);
 	release_firmware(cfg);
+	complete(&data->update_cfg_completion);
+	mutex_lock(&data->lock);
+	if (data->e_state != MXT_STATE_GOING_AWAY)
+		data->e_state = MXT_STATE_READY;
+	mutex_unlock(&data->lock);
 }
 
 static int mxt_bootloader_status(struct mxt_data *data)
@@ -3109,6 +3142,15 @@ static int mxt_initialize(struct mxt_data *data)
 		goto err_free_sysfs;
 
 	if (data->cfg_name) {
+		mutex_lock(&data->lock);
+		if (data->e_state != MXT_STATE_GOING_AWAY) {
+			data->e_state = MXT_STATE_UPDATING_CONFIG_ASYNC;
+		} else {
+			mutex_unlock(&data->lock);
+			return -EBUSY;
+		}
+		reinit_completion(&data->update_cfg_completion);
+		mutex_unlock(&data->lock);
 		error = request_firmware_nowait(THIS_MODULE, true,
 						data->cfg_name,
 						&client->dev,
@@ -3888,30 +3930,58 @@ static int mxt_update_file_name(struct device *dev, char **file_name,
 	return 0;
 }
 
+static int mxt_process_operation(struct mxt_data *data,
+				 enum mxt_cmd cmd,
+				 void *cmd_data);
+
 static ssize_t mxt_update_fw_store(struct device *dev,
-					struct device_attribute *attr,
-					const char *buf, size_t count)
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
 {
 	struct mxt_data *data = dev_get_drvdata(dev);
+	char *filename = NULL;
+	int ret;
+
+	ret = mxt_update_file_name(dev, &filename, buf, count);
+	if (ret)
+		goto out;
+
+	ret = mxt_process_operation(data, UPDATE_FW, filename);
+	kfree(filename);
+
+	if (ret)
+		goto out;
+
+	return count;
+out:
+	return ret;
+}
+
+static int mxt_fw_update(struct mxt_data *data,
+			 const char *filename)
+{
+	struct device *dev = &data->client->dev;
+	unsigned int len = 0;
 	int error;
 
-	error = mxt_update_file_name(dev, &data->fw_name, buf, count);
+	len = strlen(filename);
+	error = mxt_update_file_name(dev, &data->fw_name, filename, len);
 	if (error)
 		return error;
 
 	error = mxt_load_fw(dev);
 	if (error) {
 		dev_err(dev, "The firmware update failed(%d)\n", error);
-		count = error;
-	} else {
-		dev_info(dev, "The firmware update succeeded\n");
-
-		error = mxt_initialize(data);
-		if (error)
-			return error;
+		return error;
 	}
 
-	return count;
+	error = mxt_initialize(data);
+	if (error)
+		return error;
+
+	dev_info(dev, "The firmware update succeeded\n");
+
+	return error;
 }
 
 static ssize_t mxt_update_cfg_store(struct device *dev,
@@ -3919,14 +3989,38 @@ static ssize_t mxt_update_cfg_store(struct device *dev,
 		const char *buf, size_t count)
 {
 	struct mxt_data *data = dev_get_drvdata(dev);
+	char *filename = NULL;
+	int ret;
+
+	ret = mxt_update_file_name(dev, &filename, buf, count);
+	if (ret)
+		goto out;
+
+	ret = mxt_process_operation(data, UPDATE_CFG, filename);
+	kfree(filename);
+
+	if (ret)
+		goto out;
+
+	return count;
+out:
+	return ret;
+}
+
+static int mxt_cfg_update(struct mxt_data *data,
+			  char *filename)
+{
+	struct device *dev = &data->client->dev;
 	const struct firmware *cfg;
+	unsigned int len = 0;
 	int ret;
 
-	ret = mxt_update_file_name(dev, &data->cfg_name, buf, count);
+	len = strlen(filename);
+	ret = mxt_update_file_name(dev, &data->cfg_name, filename, len);
 	if (ret)
 		return ret;
 
-	ret = request_firmware(&cfg, data->cfg_name, dev);
+	ret = request_firmware(&cfg, data->cfg_name, &data->client->dev);
 	if (ret < 0) {
 		dev_err(dev, "Failure to request config file %s\n",
 			data->cfg_name);
@@ -3934,8 +4028,6 @@ static ssize_t mxt_update_cfg_store(struct device *dev,
 		goto out;
 	}
 
-	data->updating_config = true;
-
 	mxt_free_input_device(data);
 
 	if (data->suspended) {
@@ -3951,15 +4043,8 @@ static ssize_t mxt_update_cfg_store(struct device *dev,
 	}
 
 	ret = mxt_configure_objects(data, cfg);
-	if (ret)
-		goto release;
-
-	ret = count;
-
-release:
 	release_firmware(cfg);
 out:
-	data->updating_config = false;
 	return ret;
 }
 
@@ -4296,8 +4381,17 @@ static int mxt_start(struct mxt_data *data)
 {
 	int ret = 0;
 
-	if (!data->suspended || data->in_bootloader)
+	mutex_lock(&data->lock);
+	if (!data->suspended) {
+		mutex_unlock(&data->lock);
 		return 0;
+	}
+	if (data->in_bootloader || data->e_state != MXT_STATE_READY) {
+		mutex_unlock(&data->lock);
+		return -EBUSY;
+	}
+	data->e_state = MXT_STATE_START;
+	mutex_unlock(&data->lock);
 
 	switch (data->suspend_mode) {
 	case MXT_SUSPEND_T9_CTRL:
@@ -4341,8 +4435,12 @@ static int mxt_start(struct mxt_data *data)
 		ret = mxt_acquire_irq(data);
 	}
 
+	mutex_lock(&data->lock);
 	if (!ret)
 		data->suspended = false;
+	if (data->e_state != MXT_STATE_GOING_AWAY)
+		data->e_state = MXT_STATE_READY;
+	mutex_unlock(&data->lock);
 
 	return ret;
 }
@@ -4351,8 +4449,19 @@ static int mxt_stop(struct mxt_data *data)
 {
 	int ret;
 
-	if (data->suspended || data->in_bootloader || data->updating_config)
+	mutex_lock(&data->lock);
+	if (data->suspended) {
+		mutex_unlock(&data->lock);
 		return 0;
+	}
+	if (data->in_bootloader || (data->e_state != MXT_STATE_READY &&
+		data->e_state != MXT_STATE_GOING_AWAY)) {
+		mutex_unlock(&data->lock);
+		return -EBUSY;
+	}
+	if (data->e_state != MXT_STATE_GOING_AWAY)
+		data->e_state = MXT_STATE_STOP;
+	mutex_unlock(&data->lock);
 
 	switch (data->suspend_mode) {
 	case MXT_SUSPEND_T9_CTRL:
@@ -4382,8 +4491,15 @@ static int mxt_stop(struct mxt_data *data)
 		break;
 	}
 
+	mutex_lock(&data->lock);
 	data->suspended = true;
+
+	if (data->e_state != MXT_STATE_GOING_AWAY)
+		data->e_state = MXT_STATE_READY;
+	mutex_unlock(&data->lock);
+
 	return 0;
+
 }
 
 static int mxt_input_open(struct input_dev *dev)
@@ -4637,6 +4753,8 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	snprintf(data->phys, sizeof(data->phys), "i2c-%u-%04x/input0",
 		 client->adapter->nr, client->addr);
 
+	mutex_init(&data->lock);
+
 	irq_set_status_flags(client->irq, IRQ_DISABLE_UNLAZY);
 
 	data->client = client;
@@ -4645,6 +4763,7 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	init_completion(&data->chg_completion);
 	init_completion(&data->reset_completion);
 	init_completion(&data->crc_completion);
+	init_completion(&data->update_cfg_completion);
 	mutex_init(&data->debug_msg_lock);
 
 	data->suspend_mode = dmi_check_system(chromebook_T9_suspend_dmi) ?
@@ -4743,6 +4862,18 @@ static int mxt_remove(struct i2c_client *client)
 {
 	struct mxt_data *data = i2c_get_clientdata(client);
 
+	mutex_lock(&data->lock);
+	if (data->e_state == MXT_STATE_UPDATING_CONFIG_ASYNC ||
+	    data->e_state == MXT_STATE_UPDATING_CONFIG) {
+		data->e_state = MXT_STATE_GOING_AWAY;
+		mutex_unlock(&data->lock);
+		mxt_wait_for_completion(data, &data->update_cfg_completion,
+					MXT_CONFIG_TIMEOUT);
+	} else {
+		data->e_state = MXT_STATE_GOING_AWAY;
+		mutex_unlock(&data->lock);
+	}
+
 	disable_irq(data->irq);
 	sysfs_remove_group(&client->dev.kobj, &mxt_fw_attr_group);
 	if (data->reset_gpio) {
@@ -4801,6 +4932,45 @@ static int __maybe_unused mxt_resume(struct device *dev)
 	return ret;
 }
 
+static int mxt_process_operation(struct mxt_data *data,
+				 enum mxt_cmd cmd,
+				 void *cmd_data)
+{
+	int ret = 0;
+
+	mutex_lock(&data->lock);
+	if (data->e_state != MXT_STATE_READY) {
+		mutex_unlock(&data->lock);
+		dev_err(&data->client->dev, "Atmel touch device is shutting down\n");
+		return -EBUSY;
+	}
+	data->e_state = MXT_STATE_UPDATING_CONFIG;
+	reinit_completion(&data->update_cfg_completion);
+	mutex_unlock(&data->lock);
+
+	switch (cmd) {
+	case UPDATE_CFG:
+	case UPDATE_FW:
+		if (cmd == UPDATE_CFG)
+			ret = mxt_cfg_update(data, (char *)cmd_data);
+		else
+			ret = mxt_fw_update(data, (char *)cmd_data);
+		break;
+
+	default:
+		break;
+	}
+	mutex_lock(&data->lock);
+	if (data->e_state != MXT_STATE_UPDATING_CONFIG_ASYNC) {
+		complete(&data->update_cfg_completion);
+		if (data->e_state != MXT_STATE_GOING_AWAY)
+			data->e_state = MXT_STATE_READY;
+	}
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
 static SIMPLE_DEV_PM_OPS(mxt_pm_ops, mxt_suspend, mxt_resume);
 
 static const struct of_device_id mxt_of_match[] = {
-- 
2.19.2

^ permalink raw reply related

* [PATCH v1 63/63] Input: atmel_mxt_ts - Fix compilation warning
From: Jiada Wang @ 2019-08-16  8:39 UTC (permalink / raw)
  To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190816083902.19659-1-jiada_wang@mentor.com>

fix "make W=1" compilation warnings from Atmel driver
as per the compilation logs.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 6e5fad91c379..063565511ae1 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2056,7 +2056,7 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data, struct mxt_cfg *cfg)
 
 			byte_offset = reg + i - cfg->start_ofs;
 
-			if (byte_offset >= 0 && byte_offset < cfg->mem_size) {
+			if (byte_offset < cfg->mem_size) {
 				*(cfg->mem + byte_offset) = val;
 			} else {
 				dev_err(dev, "Bad object: reg:%d, T%d, ofs=%d\n",
-- 
2.19.2

^ permalink raw reply related

* Re: [PATCH v4 3/9] nvmem: core: add nvmem_device_find
From: Thomas Bogendoerfer @ 2019-08-16 14:09 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
	Lee Jones, David S. Miller, Alessandro Zummo, Alexandre Belloni,
	Greg Kroah-Hartman, Jiri Slaby, Evgeniy Polyakov, linux-mips,
	linux-kernel, linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <31d680ee-ddb3-8536-c915-576222d263e1@linaro.org>

On Wed, Aug 14, 2019 at 01:52:49PM +0100, Srinivas Kandagatla wrote:
> On 14/08/2019 12:46, Thomas Bogendoerfer wrote:
> >On Tue, 13 Aug 2019 10:40:34 +0100
> >Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> >>On 09/08/2019 11:32, Thomas Bogendoerfer wrote:
> >>>nvmem_device_find provides a way to search for nvmem devices with
> >>>the help of a match function simlair to bus_find_device.
> >>>
> >>>Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> >>>---
> >>>   drivers/nvmem/core.c           | 62 ++++++++++++++++++++++--------------------
> >>>   include/linux/nvmem-consumer.h |  9 ++++++
> >>>   2 files changed, 41 insertions(+), 30 deletions(-)
> >>
> >>Have you considered using nvmem_register_notifier() ?
> >
> >yes, that was the first idea. But then I realized I need to build up
> >a private database of information already present in nvmem bus. So I
> >looked for a way to retrieve it from there. Unfortunately I couldn't
> >use bus_find_device directly, because nvmem_bus_type and struct nvmem_device
> >is hidden. So I refactured the lookup code and added a more universal
> >lookup function, which fits my needs and should be usable for more.
> I see your point.
> 
> overall the patch as it is look good, but recently we added more generic
> lookups for DT node, looks like part of your patch is un-doing generic
> device name lookup.
> 
> DT node match lookup is in https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/log/?h=generic_lookup_helpers

these patches are not in Linus tree, yet. I guess they will show up
in 5.4. No idea how to deal with it right now, do you ?

> Other missing bit is adding this api to documentation in
> ./Documentation/driver-api/nvmem.rst

ok, will do.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

^ permalink raw reply

* Re: [PATCH v1 01/63] Input: introduce input_mt_report_slot_inactive
From: Dmitry Torokhov @ 2019-08-16 17:12 UTC (permalink / raw)
  To: Jiada Wang; +Cc: nick, linux-input, linux-kernel, george_davis
In-Reply-To: <20190816082952.17985-2-jiada_wang@mentor.com>

Hi Jiada,

On Fri, Aug 16, 2019 at 05:28:50PM +0900, Jiada Wang wrote:
> input_mt_report_slot_state() ignores the tool when the slot is closed.
> which has caused a bit of confusion.
> This patch introduces input_mt_report_slot_inactive() to report slot
> inactive state.
> replaces all input_mt_report_slot_state() with input_mt_report_slot_inactive()
> in case of close of slot, also some other related changes.

This seems to actually contain 2 changes:

- switching to use return value of input_mt_report_slot_state()
- actually introducing and using input_mt_report_slot_inactive()

Please split them.

>  
> +/**
> + * input_mt_report_slot_inactive() - report contact inactive state
> + * @dev: input device with allocated MT slots
> + *
> + * Reports the contact inactive state via ABS_MT_TRACKING_ID
> + *
> + */
> +void input_mt_report_slot_inactive(struct input_dev *dev)
> +{
> +	input_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> +}
> +EXPORT_SYMBOL(input_mt_report_slot_inactive);

I think this should be a static inline, there is no need to have out of
line implementation for a 1-liner.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v1 03/63] Input: atmel_mxt_ts - only read messages in mxt_acquire_irq() when necessary
From: Dmitry Torokhov @ 2019-08-16 17:16 UTC (permalink / raw)
  To: Jiada Wang; +Cc: nick, linux-input, linux-kernel, george_davis
In-Reply-To: <20190816082952.17985-4-jiada_wang@mentor.com>

On Fri, Aug 16, 2019 at 05:28:52PM +0900, Jiada Wang wrote:
> From: Nick Dyer <nick.dyer@itdev.co.uk>
> 
> The workaround of reading all messages until an invalid is received is a
> way of forcing the CHG line high, which means that when using
> edge-triggered interrupts the interrupt can be acquired.
> 
> With level-triggered interrupts the workaround is unnecessary.
> 
> Also, most recent maXTouch chips have a feature called RETRIGEN which, when
> enabled, reasserts the interrupt line every cycle if there are messages
> waiting. This also makes the workaround unnecessary.
> 
> Note: the RETRIGEN feature is only in some firmware versions/chips, it's
> not valid simply to enable the bit.

Instead of trying to work around of misconfiguration for IRQ/firmware,
can we simply error out of probe if we see a level interrupt with
!RETRIGEN firmware?

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v1 04/63] Input: atmel_mxt_ts - split large i2c transfers into blocks
From: Dmitry Torokhov @ 2019-08-16 17:18 UTC (permalink / raw)
  To: Jiada Wang; +Cc: nick, linux-input, linux-kernel, george_davis
In-Reply-To: <20190816082952.17985-5-jiada_wang@mentor.com>

On Fri, Aug 16, 2019 at 05:28:53PM +0900, Jiada Wang wrote:
> From: Nick Dyer <nick.dyer@itdev.co.uk>
> 
> On some firmware variants, the size of the info block exceeds what can
> be read in a single transfer.
> 
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
> (cherry picked from ndyer/linux/for-upstream commit 74c4f5277cfa403d43fafc404119dc57a08677db)
> [gdavis: Forward port and fix conflicts due to v4.14.51 commit
> 	 960fe000b1d3 ("Input: atmel_mxt_ts - fix the firmware
> 	 update").]
> Signed-off-by: George G. Davis <george_davis@mentor.com>
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 27 +++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 9b165d23e092..2d70ddf71cd9 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -40,7 +40,7 @@
>  #define MXT_OBJECT_START	0x07
>  #define MXT_OBJECT_SIZE		6
>  #define MXT_INFO_CHECKSUM_SIZE	3
> -#define MXT_MAX_BLOCK_WRITE	256
> +#define MXT_MAX_BLOCK_WRITE	255
>  
>  /* Object types */
>  #define MXT_DEBUG_DIAGNOSTIC_T37	37
> @@ -659,6 +659,27 @@ static int __mxt_read_reg(struct i2c_client *client,
>  	return ret;
>  }
>  
> +static int mxt_read_blks(struct mxt_data *data, u16 start, u16 count, u8 *buf)

Can we call this __mxt_read_reg() and the original read reg call
__mxt_read_chunk()?

> +{
> +	u16 offset = 0;
> +	int error;
> +	u16 size;
> +
> +	while (offset < count) {
> +		size = min(MXT_MAX_BLOCK_WRITE, count - offset);
> +
> +		error = __mxt_read_reg(data->client,
> +				       start + offset,
> +				       size, buf + offset);
> +		if (error)
> +			return error;
> +
> +		offset += size;
> +	}
> +
> +	return 0;
> +}

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v1 36/63] Input: atmel_mxt_ts - configure and use gpios as real gpios
From: Dmitry Torokhov @ 2019-08-16 17:24 UTC (permalink / raw)
  To: Jiada Wang; +Cc: nick, linux-input, linux-kernel, george_davis
In-Reply-To: <20190816083525.19071-2-jiada_wang@mentor.com>

On Fri, Aug 16, 2019 at 05:34:58PM +0900, Jiada Wang wrote:
> From: Kautuk Consul <kautuk_consul@mentor.com>
> 
> The upstream Atmel mXT driver implementation seems to handle the
> T19 GPIO/PWM object as a key pad. Keys can be defined in the
> device tree ("linux,gpio-keymap") and will be transported as key
> events to the Linux input device if GPIO state changes.
> 
> With our hardware, the GPIO pins of the touch controller are
> connected to a PWM/backlight controller and used as supervision
> inputs. We like to read the status of the pins by a script or an
> application in the sysfs.
> 
> Adding newer sysfs entries which shall be placed in the input
> class directory eg:
> /sys/class/input/input<n>/backlight_error1

No, if you want to export GPIO lines for external use create a gpiochip
instance and register it with GPIO subsystem. No ad-hoc sysfs please.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v1 39/63] Input: touchscreen: Atmel: Add device tree support for T15 key array objects
From: Dmitry Torokhov @ 2019-08-16 17:25 UTC (permalink / raw)
  To: Jiada Wang; +Cc: nick, linux-input, linux-kernel, george_davis
In-Reply-To: <20190816083525.19071-5-jiada_wang@mentor.com>

On Fri, Aug 16, 2019 at 05:35:01PM +0900, Jiada Wang wrote:
> From: Daniel Gong <Zhanli.Gong@cn.bosch.com>

This should be with the code adding T15 handling.

> 
> Signed-off-by: Daniel Gong <Zhanli.Gong@cn.bosch.com>
> Signed-off-by: George G. Davis <george_davis@mentor.com>
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 29 ++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index be63002c2b31..3b9544c0a209 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -4143,10 +4143,12 @@ static int mxt_parse_device_properties(struct mxt_data *data)
>  {
>  	static const char keymap_property[] = "linux,gpio-keymap";
>  	static const char gpios_property[] = "atmel,gpios";
> +	static const char buttons_property[] = "atmel,key-buttons";
>  	struct device *dev = &data->client->dev;
>  	struct device_node *np = dev ? dev->of_node : NULL;
>  	struct device_node *np_gpio;
>  	u32 *keymap;
> +	u32 *buttonmap;
>  	int n_keys;
>  	int error;
>  
> @@ -4181,6 +4183,33 @@ static int mxt_parse_device_properties(struct mxt_data *data)
>  		data->t19_num_keys = n_keys;
>  	}
>  
> +	if (device_property_present(dev, buttons_property)) {
> +		n_keys = device_property_read_u32_array(dev, buttons_property,
> +							NULL, 0);
> +		if (n_keys <= 0) {
> +			error = n_keys < 0 ? n_keys : -EINVAL;
> +			dev_err(dev, "invalid/malformed '%s' property: %d\n",
> +				buttons_property, error);
> +			return error;
> +		}
> +
> +		buttonmap = devm_kmalloc_array(dev, n_keys, sizeof(*buttonmap),
> +					       GFP_KERNEL);
> +		if (!buttonmap)
> +			return -ENOMEM;
> +
> +		error = device_property_read_u32_array(dev, buttons_property,
> +						       buttonmap, n_keys);
> +		if (error) {
> +			dev_err(dev, "failed to parse '%s' property: %d\n",
> +				buttons_property, error);
> +			return error;
> +		}
> +
> +		data->t15_keymap = buttonmap;
> +		data->t15_num_keys = n_keys;
> +	}
> +
>  	device_property_read_u32(dev, "atmel,suspend-mode", &data->suspend_mode);
>  
>  	np_gpio = of_get_child_by_name(np, gpios_property);
> -- 
> 2.19.2
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v1 41/63] Input: touchscreen: Atmel: Enable IRQ_DISABLE_UNLAZY flag for interrupt
From: Dmitry Torokhov @ 2019-08-16 17:26 UTC (permalink / raw)
  To: Jiada Wang; +Cc: nick, linux-input, linux-kernel, george_davis
In-Reply-To: <20190816083558.19189-2-jiada_wang@mentor.com>

On Fri, Aug 16, 2019 at 05:35:36PM +0900, Jiada Wang wrote:
> From: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com>
> 
> The de-/serializer driver has defined only irq_mask "ds90ub927_irq_mask" and
> irq_unmask "ds90ub927_irq_unmask" callback functions. And de-/serializer
> driver doesn't implement the irq_disable and irq_enable callback functions.
> Hence inorder to invoke irq_mask callback function when disable_irq_nosync is
> called the IRQ_DISABLE_UNLAZY interrupt flag should be set. If not the
> disable_irq_nosync will just increment the depth field in the irq
> descriptor only once as shown below.
> 
> disable_irq_nosync
>  __disable_irq_nosync
>   __disable_irq (desc->depth++)
>    irq_disable
>     if irq_disable present -----------> if IRQ_DISABLE_UNLAZYflag set
>              |                  no                  |
>          yes |                                  yes |
>              |                                      |
>      desc->irq_data.chip->irq_disable   desc->irq_data.chip->irq_unmask
>                                          (ds90ub927_irq_mask)
>                                           disable_irq
>                                            __disable_irq_nosync
>                                             __disable_irq
> (desc->depth++)
> But the enable_irq will try to decrement the depth field twice which generates
> the backtrace stating "Unbalanced enable for irq 293". This is because there is
> no IRQ_DISABLE_UNLAZY flag check while calling irq_unmask callback function
> of the "ds90ub927_irq_unmask" de-/serializer via enable_irq.
> 
> enable_irq
>  __enable_irq (desc->depth--)
>   irq_enable
>    if irq_enable present -------------> desc->irq_data.chip->irq_unmask
>               |                no        (ds90ub927_irq_unmask)
>           yes |                            enable_irq
>               |                             __enable_irq (desc->depth--)
>     (desc->irq_data.chip->irq_enable)

I'd prefer if we instead did not use the disable_irq_nosync() in the
driver.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v1 31/63] Input: atmel_mxt_ts - add memory access interface via sysfs
From: Dmitry Torokhov @ 2019-08-16 17:29 UTC (permalink / raw)
  To: Jiada Wang; +Cc: nick, linux-input, linux-kernel, george_davis
In-Reply-To: <20190816083451.18947-2-jiada_wang@mentor.com>

On Fri, Aug 16, 2019 at 05:34:19PM +0900, Jiada Wang wrote:
> From: Nick Dyer <nick.dyer@itdev.co.uk>
> 
> Atmel maXTouch chips can be addressed via an "Object Based Protocol" which
> defines how i2c registers are mapped to different functions within the
> chips. This interface exposes the register map and allows user-space
> utilities to inspect and alter object configuration, and to view diagnostic
> data, while the device is running.

I'd say if we want this we should look into write support in regmap and
switching the driver over.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v1 00/63] atmel_mxt_ts misc
From: Dmitry Torokhov @ 2019-08-16 17:32 UTC (permalink / raw)
  To: Jiada Wang; +Cc: nick, linux-input, linux-kernel, george_davis
In-Reply-To: <20190816082952.17985-1-jiada_wang@mentor.com>

Hi Jiada,

On Fri, Aug 16, 2019 at 05:28:49PM +0900, Jiada Wang wrote:
> This patch-set forward ports Nick Dyer's work in ndyer/linux github repository
> as long as some other features and fixes

I see a lot of chages that are fixups for older changes in your series.
They need to be identified and squashed together, as we do not need to
be aware of your development history.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v1 06/63] Input: atmel_mxt_ts - output status from T42 Touch Suppression
From: Dmitry Torokhov @ 2019-08-16 17:34 UTC (permalink / raw)
  To: Jiada Wang; +Cc: nick, linux-input, linux-kernel, george_davis
In-Reply-To: <20190816083130.18250-2-jiada_wang@mentor.com>

On Fri, Aug 16, 2019 at 05:30:33PM +0900, Jiada Wang wrote:
> From: Nick Dyer <nick.dyer@itdev.co.uk>
> 
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
> Acked-by: Benson Leung <bleung@chromium.org>
> Acked-by: Yufeng Shen <miletus@chromium.org>
> (cherry picked from ndyer/linux/for-upstream commit ab95b5a309999d2c098daaa9f88d9fcfae7eb516)
> Signed-off-by: George G. Davis <george_davis@mentor.com>
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 25 ++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index a75c35c6f9f9..9226ec528adf 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -155,6 +155,9 @@ struct t37_debug {
>  #define MXT_RESET_VALUE		0x01
>  #define MXT_BACKUP_VALUE	0x55
>  
> +/* Define for MXT_PROCI_TOUCHSUPPRESSION_T42 */
> +#define MXT_T42_MSG_TCHSUP	BIT(0)
> +
>  /* T100 Multiple Touch Touchscreen */
>  #define MXT_T100_CTRL		0
>  #define MXT_T100_CFG1		1
> @@ -323,6 +326,8 @@ struct mxt_data {
>  	u8 T9_reportid_max;
>  	u16 T18_address;
>  	u8 T19_reportid;
> +	u8 T42_reportid_min;
> +	u8 T42_reportid_max;
>  	u16 T44_address;
>  	u8 T48_reportid;
>  	u8 T100_reportid_min;
> @@ -978,6 +983,17 @@ static void mxt_proc_t100_message(struct mxt_data *data, u8 *message)
>  	data->update_input = true;
>  }
>  
> +static void mxt_proc_t42_messages(struct mxt_data *data, u8 *msg)
> +{
> +	struct device *dev = &data->client->dev;
> +	u8 status = msg[1];
> +
> +	if (status & MXT_T42_MSG_TCHSUP)
> +		dev_info(dev, "T42 suppress\n");
> +	else
> +		dev_info(dev, "T42 normal\n");

dev_dbg(). There is no need to flood the logs with this. I'd assume this
is for assisting in bringup. Should there be some more generic way of
monitoring the status?

-- 
Dmitry

^ permalink raw reply

* [PATCH] HID: wacom: correct misreported EKR ring values
From: Aaron Armstrong Skomra @ 2019-08-16 19:00 UTC (permalink / raw)
  To: linux-input, jikos, benjamin.tissoires, ping.cheng, jason.gerecke
  Cc: Aaron Armstrong Skomra, # v4 . 3+

The EKR ring claims a range of 0 to 71 but actually reports
values 1 to 72. The ring is used in relative mode so this
change should not affect users.

Signed-off-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
Fixes: 72b236d60218f ("HID: wacom: Add support for Express Key Remote.")
Cc: <stable@vger.kernel.org> # v4.3+
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
---
 drivers/hid/wacom_wac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index abc17f2c8ef0..e314a7564236 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1098,7 +1098,7 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
 	input_report_key(input, BTN_BASE2, (data[11] & 0x02));
 
 	if (data[12] & 0x80)
-		input_report_abs(input, ABS_WHEEL, (data[12] & 0x7f));
+		input_report_abs(input, ABS_WHEEL, (data[12] & 0x7f) - 1);
 	else
 		input_report_abs(input, ABS_WHEEL, 0);
 
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH 2/2] HID: wacom: do not call hid_set_drvdata(hdev, NULL)
From: Jason Gerecke @ 2019-08-16 22:03 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Bruno Prémont, Jonathan Cameron, Srinivas Pandruvada,
	Ping Cheng, Jason Gerecke, Jiri Kosina, Linux Input, LKML
In-Reply-To: <20190812162740.15898-3-benjamin.tissoires@redhat.com>

On Mon, Aug 12, 2019 at 9:29 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> This is a common pattern in the HID drivers to reset the drvdata.
> However, this is actually already handled by driver core, so there
> is no need to do it manually.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Acked-by: Jason Gerecke <jason.gerecke@wacom.com>

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....


> ---
>  drivers/hid/wacom_sys.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 53bddb50aeba..69ccfdd51a6f 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2718,14 +2718,12 @@ static int wacom_probe(struct hid_device *hdev,
>         wacom_wac->features = *((struct wacom_features *)id->driver_data);
>         features = &wacom_wac->features;
>
> -       if (features->check_for_hid_type && features->hid_type != hdev->type) {
> -               error = -ENODEV;
> -               goto fail;
> -       }
> +       if (features->check_for_hid_type && features->hid_type != hdev->type)
> +               return -ENODEV;
>
>         error = kfifo_alloc(&wacom_wac->pen_fifo, WACOM_PKGLEN_MAX, GFP_KERNEL);
>         if (error)
> -               goto fail;
> +               return error;
>
>         wacom_wac->hid_data.inputmode = -1;
>         wacom_wac->mode_report = -1;
> @@ -2743,12 +2741,12 @@ static int wacom_probe(struct hid_device *hdev,
>         error = hid_parse(hdev);
>         if (error) {
>                 hid_err(hdev, "parse failed\n");
> -               goto fail;
> +               return error;
>         }
>
>         error = wacom_parse_and_register(wacom, false);
>         if (error)
> -               goto fail;
> +               return error;
>
>         if (hdev->bus == BUS_BLUETOOTH) {
>                 error = device_create_file(&hdev->dev, &dev_attr_speed);
> @@ -2759,10 +2757,6 @@ static int wacom_probe(struct hid_device *hdev,
>         }
>
>         return 0;
> -
> -fail:
> -       hid_set_drvdata(hdev, NULL);
> -       return error;
>  }
>
>  static void wacom_remove(struct hid_device *hdev)
> @@ -2791,8 +2785,6 @@ static void wacom_remove(struct hid_device *hdev)
>                 wacom_release_resources(wacom);
>
>         kfifo_free(&wacom_wac->pen_fifo);
> -
> -       hid_set_drvdata(hdev, NULL);
>  }
>
>  #ifdef CONFIG_PM
> --
> 2.19.2
>

^ permalink raw reply

* [PATCH 0/4] Add support for updated vmware hypercall instruction
From: Thomas Hellström (VMware) @ 2019-08-18 14:33 UTC (permalink / raw)
  To: linux-kernel, pv-drivers
  Cc: Thomas Hellström, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, x86, Dmitry Torokhov,
	linux-input

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: <x86@kernel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: <linux-input@vger.kernel.org>

VMware has started using "vmcall" / "vmmcall" instead of an inl instruction
for the "backdoor" interface. This series detects support for those
instructions.
Outside of the platform code we use the "ALTERNATIVES" self-patching
mechanism similarly to how this is done with KVM.
Unfortunately we need two x86 cpu feature flags for this, since we need
the default instruction to be "inl". IIRC the vmmouse driver is used by
other virtualization solutions than VMware, and those might break if
they encounter any of the other instructions.

^ permalink raw reply

* [PATCH 4/4] input/vmmouse: Update the backdoor call with support for new instructions
From: Thomas Hellström (VMware) @ 2019-08-18 14:33 UTC (permalink / raw)
  To: linux-kernel, pv-drivers
  Cc: Thomas Hellstrom, Dmitry Torokhov, linux-input, Doug Covelli
In-Reply-To: <20190818143316.4906-1-thomas_os@shipmail.org>

From: Thomas Hellstrom <thellstrom@vmware.com>

Use the definition provided by include/asm/vmware.h

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: <linux-input@vger.kernel.org>
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Doug Covelli <dcovelli@vmware.com>
---
 drivers/input/mouse/vmmouse.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
index 871e5b5ab129..0c7707c7bede 100644
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -16,6 +16,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <asm/hypervisor.h>
+#include <asm/vmware.h>
 
 #include "psmouse.h"
 #include "vmmouse.h"
@@ -84,7 +85,7 @@ struct vmmouse_data {
 #define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4)	\
 ({							\
 	unsigned long __dummy1, __dummy2;		\
-	__asm__ __volatile__ ("inl %%dx" :		\
+	__asm__ __volatile__ (VMWARE_HYPERCALL :	\
 		"=a"(out1),				\
 		"=b"(out2),				\
 		"=c"(out3),				\
-- 
2.20.1

^ permalink raw reply related

* Re: [PATCH 0/4] Add support for updated vmware hypercall instruction
From: Borislav Petkov @ 2019-08-18 18:09 UTC (permalink / raw)
  To: Thomas Hellström (VMware)
  Cc: linux-kernel, pv-drivers, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Dmitry Torokhov, linux-input
In-Reply-To: <20190818143316.4906-1-thomas_os@shipmail.org>

On Sun, Aug 18, 2019 at 04:33:12PM +0200, Thomas Hellström (VMware) wrote:
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: <x86@kernel.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: <linux-input@vger.kernel.org>

In the future, when you CC people on patches, pls CC them for all
patches - not only a subset. Had to fish out 3/4 and 4/4 from lkml.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* Re: [PATCH 0/4] Add support for updated vmware hypercall instruction
From: Thomas Hellström (VMware) @ 2019-08-18 19:02 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-kernel, pv-drivers, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Dmitry Torokhov, linux-input
In-Reply-To: <20190818180929.GA29353@zn.tnic>

On 8/18/19 8:09 PM, Borislav Petkov wrote:
> On Sun, Aug 18, 2019 at 04:33:12PM +0200, Thomas Hellström (VMware) wrote:
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: <x86@kernel.org>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Cc: <linux-input@vger.kernel.org>
> In the future, when you CC people on patches, pls CC them for all
> patches - not only a subset. Had to fish out 3/4 and 4/4 from lkml.
>
> Thx.
>
Hmm. Ok. I'll keep that in mind.
I was trying to be clever and avoid spamming people with stuff they 
might not care about.

Thanks,
/Thomas

^ permalink raw reply

* Re: [PATCH 0/4] Add support for updated vmware hypercall instruction
From: Borislav Petkov @ 2019-08-18 19:37 UTC (permalink / raw)
  To: Thomas Hellström (VMware)
  Cc: linux-kernel, pv-drivers, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Dmitry Torokhov, linux-input
In-Reply-To: <c0f32d6b-3b6a-f931-071f-9fb308efba00@shipmail.org>

On Sun, Aug 18, 2019 at 09:02:04PM +0200, Thomas Hellström (VMware) wrote:
> I was trying to be clever and avoid spamming people with stuff they might
> not care about.

Sure but if, for example, you're adding VMWARE_HYPERCALL in patch 2 and
the users are in the following patches, it would be helpful if I could
see them too. That is one good reason to CC everybody with the whole
patchset.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* Re: [PATCH v2 0/3] HID: intel-ish-hid: support s2idle and s3 in ish_suspend()
From: Srinivas Pandruvada @ 2019-08-19  2:52 UTC (permalink / raw)
  To: Zhang Lixu, linux-input, jikos, benjamin.tissoires; +Cc: linux-kernel
In-Reply-To: <20190808102113.27802-1-lixu.zhang@intel.com>

On Thu, 2019-08-08 at 18:21 +0800, Zhang Lixu wrote:
> Currently, the NO_D3 flag is set in ish_probe(), the intel-ish-ipc
> driver
> puts the ISH into D0i3 when system enter both suspend-to-idle(S0ix)
> and
> suspend-to-mem(S3). These patches are to put the ISH into D3 when
> system
> enter S3 and put the ISH into D0i3 when system enter S0ix.
> 
> I tested these patches on the following platforms:
> ISH 5.2: ICL
> ISH 5.0: CNL CFL WHL CML
> ISH 4.0: GLK
> ISH 3.0: KBL
> 
> Test steps:
> * Run the IIO Sensor tool to read the accel_3d sensor data
> * Run cmd "echo mem > /sys/power/state" to suspend-to-mem
> * Press the keyboard to wake up OS.
> * Check if the tool can get the sensor data after OS resume.
> * Run cmd "echo freeze > /sys/power/state" to suspend-to-idle
> * Press the keyboard to wake up OS.
> * Check if the tool can get the sensor data after OS resume.
> 
> Test results:
> The tool can get the accel_3d sensor data after resuming from both
> suspend-to-mem and suspend-to-idle.
> 
> Changes from v1:
> * Fix the indentation issue
> * Elaborate the reason to remove the NO_D3 flag
> * Split the PATCH v1 to three changes, and try to minimize the lines
> change
> 
> Zhang Lixu (3):
>   HID: intel-ish-hid: ipc: set NO_D3 flag only when needed
>   HID: intel-ish-hid: ipc: make ish suspend paths clear
>   HID: intel-ish-hid: ipc: check the NO_D3 flag to distinguish resume
>     paths
> 

For the series:

Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>


>  drivers/hid/intel-ish-hid/ipc/hw-ish.h  |  1 +
>  drivers/hid/intel-ish-hid/ipc/ipc.c     |  2 +-
>  drivers/hid/intel-ish-hid/ipc/pci-ish.c | 95 +++++++++++++++------
> ----
>  3 files changed, 61 insertions(+), 37 deletions(-)
> 

^ permalink raw reply

* Re: [PATCH v1 00/63] atmel_mxt_ts misc
From: Jiada Wang @ 2019-08-19  9:29 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: nick, linux-input, linux-kernel, george_davis
In-Reply-To: <20190816173209.GL121898@dtor-ws>

  HiDmitrij

On 2019/08/17 2:32, Dmitry Torokhov wrote:
> Hi Jiada,
> 
> On Fri, Aug 16, 2019 at 05:28:49PM +0900, Jiada Wang wrote:
>> This patch-set forward ports Nick Dyer's work in ndyer/linux github repository
>> as long as some other features and fixes
> 
> I see a lot of chages that are fixups for older changes in your series.
> They need to be identified and squashed together, as we do not need to
> be aware of your development history.
> 

Thanks for your comments,
in v1, I already squashed several of our internal changes,
but seems there are still some can be squashed,
I will try to squash all possible changes in v2 patch-set

Thanks,
Jiada
> Thanks.
> 

^ permalink raw reply

* [PATCH] driver:st1633: fixed multitouch incorrect coordinates
From: Dixit Parmar @ 2019-08-19 10:08 UTC (permalink / raw)
  To: dmitry.torokhov, rydberg, martink, kuninori.morimoto.gx, robh,
	matthias.fend, linux-input, linux-kernel
  Cc: Dixit Parmar

From: Dixit Parmar <dixitparmar19@gmail.com>

For Sitronix st1633 multi-touch controller driver the co-ordinates reported
for multiple fingers were wrong.

So the below mentioned bug was filed,
Bugzilla Bug ID: 204561

While reading co-ordinates from specified I2C registers, the X & Y
co-ordinates should be read from proper I2C address for particular finger as
specified in chip specific datasheet.

for single touch this logic is working fine. However, for multi-touch
scenario the logic of reading data from data buffer has issues.

This patch fixes the reading logic from data buffer.

Previous logic:
* Offset of X & Y Lower byte coordinate is increased by i no. only(by 1 Byte)
  for each finger.

New logic:
* The logic of reading X & Y Lower Byte coordinate needs to be increased
  by i+y for each time/finger.

Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
---
 drivers/input/touchscreen/st1232.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index 3492339..1139714 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -81,8 +81,10 @@ static int st1232_ts_read_data(struct st1232_ts_data *ts)
 	for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) {
 		finger[i].is_valid = buf[i + y] >> 7;
 		if (finger[i].is_valid) {
-			finger[i].x = ((buf[i + y] & 0x0070) << 4) | buf[i + 1];
-			finger[i].y = ((buf[i + y] & 0x0007) << 8) | buf[i + 2];
+			finger[i].x = ((buf[i + y] & 0x0070) << 4) |
+					buf[i + y + 1];
+			finger[i].y = ((buf[i + y] & 0x0007) << 8) |
+					buf[i + y + 2];
 
 			/* st1232 includes a z-axis / touch strength */
 			if (ts->chip_info->have_z)
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox