Re: [PATCH v11 33/56] Input: atmel_mxt_ts - delay enabling IRQ when not using regulators

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Dmitry

Thanks for your comment and test,

can you let me know which platform (board) you are using for test,
and DTS changes if you have added any.

Thanks,
Jiada

On 2020/05/11 10:05, Dmitry Osipenko wrote:
08.05.2020 08:56, Jiada Wang пишет:
The path of enabling the IRQ in the probe function is not safe in level
triggered operation, if it was already powered up and there is a message
waiting on the device (eg finger down) because the object table has not yet
been read. This forces the ISR into a hard loop.

Delay enabling the interrupt until it is first needed, by set flag
IRQ_NOAUTOEN.

Signed-off-by: Jiada Wang <jiada_wang@xxxxxxxxxx>
CC: Dmitry Osipenko <digetx@xxxxxxxxx>
---
  drivers/input/touchscreen/atmel_mxt_ts.c | 7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 7c9a738e633a..ab4eceac8fe7 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -3822,6 +3822,7 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
  		return error;
  	}
+ irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
  	error = devm_request_threaded_irq(&client->dev, client->irq,
  					  NULL, mxt_interrupt, IRQF_ONESHOT,
  					  client->name, data);
@@ -3831,17 +3832,19 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
  	}
if (data->suspend_mode == MXT_SUSPEND_REGULATOR) {
+		enable_irq(data->irq);

Hello Jiada,

This change contradicts to the commit's message since enabling IRQ until
hardware has been fully powered-on and reset is not allowed.

  		error = mxt_probe_regulators(data);
  		if (error)
  			return error;
+
+		disable_irq(data->irq);
  	} else if (data->reset_gpio) {
  		msleep(MXT_RESET_GPIO_TIME);
  		gpiod_set_value(data->reset_gpio, 1);
  		msleep(MXT_RESET_INVALID_CHG);
  	}
- disable_irq(data->irq);
-
  	error = mxt_initialize(data);
  	if (error)
  		return error;


Secondly, I gave a try to this version of the series and unfortunately
it doesn't work at all:

[  125.928709] INFO: task systemd-udevd:184 blocked for more than 61
seconds.
[  125.929130]       Not tainted
5.7.0-rc4-next-20200508-00189-g0fe7f91d4a66-dirty #2206
[  125.929474] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[  125.929900] systemd-udevd   D    0   184    173 0x00000080
[  125.929921] [<c098b995>] (__schedule) from [<c098bdc1>]
(schedule+0x65/0xc0)
[  125.929965] [<c098bdc1>] (schedule) from [<c0166ce3>]
(synchronize_irq+0x5b/0x7c)
[  125.930001] [<c0166ce3>] (synchronize_irq) from [<c067e0f9>]
(mxt_stop+0x51/0xe0)
[  125.930016] [<c067e0f9>] (mxt_stop) from [<c067e1d3>]
(mxt_input_close+0x13/0x34)
[  125.930042] [<c067e1d3>] (mxt_input_close) from [<c0664b19>]
(input_close_device+0x3d/0x5c)
[  125.930063] [<c0664b19>] (input_close_device) from [<c066b9df>]
(evdev_release+0xa7/0xbc)
[  125.930088] [<c066b9df>] (evdev_release) from [<c025a871>]
(__fput+0x91/0x198)
[  125.930121] [<c025a871>] (__fput) from [<c0136efb>]
(task_work_run+0x73/0x90)
[  125.930138] [<c0136efb>] (task_work_run) from [<c0108fa9>]
(do_work_pending+0x381/0x430)
[  125.930149] [<c0108fa9>] (do_work_pending) from [<c01000d1>]
(slow_work_pending+0x9/0x18)
[  125.930153] Exception stack(0xedd0ffb0 to 0xedd0fff8)

Please test everything properly and fix it in the next version.

BTW, it won't hurt to apply a spell-checker to the commit messages to
fix small typos.




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux