* [PATCH v2 38/49] Input: atmel_mxt_ts: return error from mxt_process_messages_until_invalid()
From: Jiada Wang @ 2019-08-27 6:30 UTC (permalink / raw)
To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190827063054.20883-1-jiada_wang@mentor.com>
From: Dean Jenkins <Dean_Jenkins@mentor.com>
mxt_process_messages_until_invalid() failed to propagate the error
code from mxt_read_and_process_messages() so return the error code.
Signed-off-by: Dean Jenkins <Dean_Jenkins@mentor.com>
Signed-off-by: Deepak Das <deepak_das@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 | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 169107413823..e9a895473ed8 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1561,6 +1561,8 @@ static int mxt_process_messages_until_invalid(struct mxt_data *data)
/* Read messages until we force an invalid */
do {
read = mxt_read_and_process_messages(data, count);
+ if (read < 0)
+ return read;
if (read < count)
return 0;
} while (--tries);
--
2.19.2
^ permalink raw reply related
* [PATCH v2 39/49] Input: Atmel: improve error handling in mxt_start()
From: Jiada Wang @ 2019-08-27 6:30 UTC (permalink / raw)
To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190827063054.20883-1-jiada_wang@mentor.com>
From: Deepak Das <deepak_das@mentor.com>
mxt_start() does not return error in any of
the failure cases which will allow input_dev->open()
to return success even in case of any failure.
This commit modifies mxt_start() to return error
in failure cases.
Signed-off-by: Deepak Das <deepak_das@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 | 31 ++++++++++++------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index e9a895473ed8..dd70f3b9678f 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -3971,12 +3971,13 @@ static int mxt_start(struct mxt_data *data)
switch (data->suspend_mode) {
case MXT_SUSPEND_T9_CTRL:
- mxt_soft_reset(data);
-
+ ret = mxt_soft_reset(data);
+ if (ret)
+ break;
/* Touch enable */
/* 0x83 = SCANEN | RPTEN | ENABLE */
- mxt_write_object(data,
- MXT_TOUCH_MULTI_T9, MXT_T9_CTRL, 0x83);
+ ret = mxt_write_object(data,
+ MXT_TOUCH_MULTI_T9, MXT_T9_CTRL, 0x83);
break;
case MXT_SUSPEND_REGULATOR:
@@ -3990,27 +3991,26 @@ static int mxt_start(struct mxt_data *data)
* Discard any touch messages still in message buffer
* from before chip went to sleep
*/
- mxt_process_messages_until_invalid(data);
+ ret = mxt_process_messages_until_invalid(data);
+ if (ret)
+ break;
ret = mxt_set_t7_power_cfg(data, MXT_POWER_CFG_RUN);
if (ret)
- return ret;
+ break;
/* Recalibrate since chip has been in deep sleep */
ret = mxt_t6_command(data, MXT_COMMAND_CALIBRATE, 1, false);
if (ret)
- return ret;
+ break;
ret = mxt_acquire_irq(data);
- if (ret)
- return ret;
-
- break;
}
- data->suspended = false;
+ if (!ret)
+ data->suspended = false;
- return 0;
+ return ret;
}
static int mxt_stop(struct mxt_data *data)
@@ -4331,6 +4331,7 @@ static int __maybe_unused mxt_resume(struct device *dev)
struct i2c_client *client = to_i2c_client(dev);
struct mxt_data *data = i2c_get_clientdata(client);
struct input_dev *input_dev = data->input_dev;
+ int ret = 0;
if (!input_dev)
return 0;
@@ -4338,11 +4339,11 @@ static int __maybe_unused mxt_resume(struct device *dev)
mutex_lock(&input_dev->mutex);
if (input_dev->users)
- mxt_start(data);
+ ret = mxt_start(data);
mutex_unlock(&input_dev->mutex);
- return 0;
+ return ret;
}
static SIMPLE_DEV_PM_OPS(mxt_pm_ops, mxt_suspend, mxt_resume);
--
2.19.2
^ permalink raw reply related
* [PATCH v2 40/49] Input: Atmel: improve error handling in mxt_initialize()
From: Jiada Wang @ 2019-08-27 6:31 UTC (permalink / raw)
To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
From: Deepak Das <deepak_das@mentor.com>
Currently mxt_initialize() tries to probe bootloader mode
even if valid bootloader address is not specified.
This commit modifies mxt_initialize() to return error
if Device is not in appmode and bootloader address is
not specified.
This commit also returns error code from mxt_send_bootloader_cmd()
in mxt_initialize().
Signed-off-by: Deepak Das <deepak_das@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 | 58 +++++++++++++++++-------
1 file changed, 41 insertions(+), 17 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index dd70f3b9678f..59533753a431 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -713,17 +713,13 @@ static int mxt_lookup_bootloader_address(struct mxt_data *data, bool retry)
return 0;
}
-static int mxt_probe_bootloader(struct mxt_data *data, bool alt_address)
+static int mxt_probe_bootloader(struct mxt_data *data)
{
struct device *dev = &data->client->dev;
int error;
u8 buf[3];
bool crc_failure, extended_id;
- error = mxt_lookup_bootloader_address(data, alt_address);
- if (error)
- return error;
-
/* Check bootloader status and version information */
error = mxt_bootloader_read(data, buf, sizeof(buf));
if (error)
@@ -2920,6 +2916,32 @@ static void mxt_config_cb(const struct firmware *cfg, void *ctx)
release_firmware(cfg);
}
+static int mxt_bootloader_status(struct mxt_data *data)
+{
+ struct i2c_client *client = data->client;
+ int error;
+
+ error = mxt_lookup_bootloader_address(data, false);
+ if (error) {
+ dev_info(&client->dev,
+ "Bootloader address is not specified\n");
+ return error;
+ }
+ /* Check bootloader state */
+ error = mxt_probe_bootloader(data);
+ if (error) {
+ dev_info(&client->dev, "Trying alternate bootloader address\n");
+ mxt_lookup_bootloader_address(data, true);
+ error = mxt_probe_bootloader(data);
+ if (error) {
+ dev_err(&client->dev,
+ "Chip is not in appmode or bootloader mode\n");
+ return error;
+ }
+ }
+ return 0;
+}
+
static int mxt_initialize(struct mxt_data *data)
{
struct i2c_client *client = data->client;
@@ -2931,16 +2953,13 @@ static int mxt_initialize(struct mxt_data *data)
if (!error)
break;
- /* Check bootloader state */
- error = mxt_probe_bootloader(data, false);
- if (error) {
- dev_info(&client->dev, "Trying alternate bootloader address\n");
- error = mxt_probe_bootloader(data, true);
- if (error) {
- /* Chip is not in appmode or bootloader mode */
- return error;
- }
- }
+ dev_info(&client->dev,
+ "info block read failed (%d), so try bootloader method\n",
+ error);
+
+ error = mxt_bootloader_status(data);
+ if (error)
+ return error;
/* OK, we are in bootloader, see if we can recover */
if (++recovery_attempts > 1) {
@@ -2954,7 +2973,9 @@ static int mxt_initialize(struct mxt_data *data)
}
/* Attempt to exit bootloader into app mode */
- mxt_send_bootloader_cmd(data, false);
+ error = mxt_send_bootloader_cmd(data, false);
+ if (error)
+ return error;
msleep(MXT_FW_RESET_TIME);
}
@@ -3646,8 +3667,11 @@ static int mxt_enter_bootloader(struct mxt_data *data)
msleep(MXT_RESET_TIME);
+ ret = mxt_lookup_bootloader_address(data, false);
+ if (ret)
+ return ret;
/* Do not need to scan since we know family ID */
- ret = mxt_probe_bootloader(data, 0);
+ ret = mxt_probe_bootloader(data);
if (ret)
return ret;
--
2.19.2
^ permalink raw reply related
* [PATCH v2 41/49] Input: Atmel: improve error handling in mxt_update_cfg()
From: Jiada Wang @ 2019-08-27 6:31 UTC (permalink / raw)
To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190827063130.20969-1-jiada_wang@mentor.com>
From: Deepak Das <deepak_das@mentor.com>
mxt_update_cfg() failed to propagate the error
code from mxt_init_t7_power_cfg() so return the error code.
Signed-off-by: Deepak Das <deepak_das@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 | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 59533753a431..716b91e6fd66 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2183,7 +2183,9 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *fw)
dev_info(dev, "Config successfully updated\n");
/* T7 config may have changed */
- mxt_init_t7_power_cfg(data);
+ ret = mxt_init_t7_power_cfg(data);
+ if (ret)
+ dev_warn(dev, "Power Config failed to update\n");
release_mem:
kfree(cfg.mem);
--
2.19.2
^ permalink raw reply related
* [PATCH v2 42/49] Input: Atmel: Improve error handling in mxt_initialize_input_device()
From: Jiada Wang @ 2019-08-27 6:31 UTC (permalink / raw)
To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190827063130.20969-1-jiada_wang@mentor.com>
From: Deepak Das <deepak_das@mentor.com>
Currently Driver probe continues with a warning message when it fails to
get the proper multitouch object configurations like TouchScreen resolution.
But Driver probe should fail in case of above scneario because it will not behave
as expected without the proper touchscreen configurations.
This commit modifies mxt_initialize_input_device() to return error when it fails
to get the proper touch screen configurations.
Signed-off-by: Deepak Das <deepak_das@mentor.com>
Signed-off-by: Dean Jenkins <Dean_Jenkins@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 | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 716b91e6fd66..41d92d765aa2 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2759,15 +2759,19 @@ static int mxt_initialize_input_device(struct mxt_data *data)
case MXT_TOUCH_MULTI_T9:
num_mt_slots = data->T9_reportid_max - data->T9_reportid_min + 1;
error = mxt_read_t9_resolution(data);
- if (error)
- dev_warn(dev, "Failed to initialize T9 resolution\n");
+ if (error) {
+ dev_err(dev, "Failed to initialize T9 resolution\n");
+ return error;
+ }
break;
case MXT_TOUCH_MULTITOUCHSCREEN_T100:
num_mt_slots = data->num_touchids;
error = mxt_read_t100_config(data);
- if (error)
- dev_warn(dev, "Failed to read T100 config\n");
+ if (error) {
+ dev_err(dev, "Failed to read T100 config\n");
+ return error;
+ }
break;
default:
--
2.19.2
^ permalink raw reply related
* [PATCH v2 43/49] Input: Atmel: handle ReportID "0x00" while processing T5 messages
From: Jiada Wang @ 2019-08-27 6:31 UTC (permalink / raw)
To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190827063130.20969-1-jiada_wang@mentor.com>
From: Deepak Das <deepak_das@mentor.com>
ReportID "0x00" is reserved by Atmel and should not be used by any
Atmel touch controller.
reportID is the first byte retrieved from T5 message payload.
Currently Atmel driver continues to process the T5 messages even if
the reportID "0x00" is returned by Touch Controller.
This commit modifies Atmel touch driver to return -EINVAL if ReportID
"0x00" is received while processing T5 messages.
Signed-off-by: Deepak Das <deepak_das@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 | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 41d92d765aa2..b6f50fee3695 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -76,6 +76,7 @@
#define MXT_PROCI_TOUCHSEQUENCELOGGER 93
#define MXT_TOUCH_MULTITOUCHSCREEN_T100 100
#define MXT_PROCI_ACTIVESTYLUS_T107 107
+#define MXT_RPTID_RESERVED 0
/* MXT_GEN_MESSAGE_T5 object */
#define MXT_RPTID_NOMSG 0xff
@@ -1381,6 +1382,11 @@ static int mxt_proc_message(struct mxt_data *data, u8 *message)
u8 report_id = message[0];
bool dump = data->debug_enabled;
+ if (report_id == MXT_RPTID_RESERVED) {
+ dev_err(&data->client->dev,
+ "Received Reserved ReportID 0x00\n");
+ return -EINVAL;
+ }
if (report_id == MXT_RPTID_NOMSG)
return 0;
@@ -1451,6 +1457,8 @@ static int mxt_read_and_process_messages(struct mxt_data *data, u8 count)
ret = mxt_proc_message(data,
data->msg_buf + data->T5_msg_size * i);
+ if (ret < 0)
+ return ret;
if (ret == 1)
num_valid++;
}
--
2.19.2
^ permalink raw reply related
* [PATCH v2 44/49] Input: Atmel: use T44 object to process T5 messages
From: Jiada Wang @ 2019-08-27 6:31 UTC (permalink / raw)
To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190827063130.20969-1-jiada_wang@mentor.com>
From: Deepak Das <deepak_das@mentor.com>
T44 object returns the count of valid T5 messages in the buffer. According
to atmel, this count should be the main criteria to read the number of T5
messages.
Following is the statement from atmel confirming the same :-
"For the readout of messages we recommend to stop after the last message
is read out from the buffer. One way to identify the amount of new messages
is to read T44. The other way is to monitor the /CHG line which indicates
independent of mode 0 or mode 1 if there are still data in the buffer.
0xFF indicates that there is no message pending anymore, but it is not
recommended to use this as the main criteria to control the
data transfer."
This commit modifies the logic to readout the T5 messages on the basis
of T44 object.
Signed-off-by: Deepak Das <deepak_das@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 | 55 +++++++++++++++---------
1 file changed, 35 insertions(+), 20 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index b6f50fee3695..58e54eb45cf0 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1486,7 +1486,7 @@ static u8 mxt_max_msg_read_count(struct mxt_data *data, u8 max_T5_msg_count)
return min(T5_msg_count_limit, max_T5_msg_count);
}
-static irqreturn_t mxt_process_messages_t44(struct mxt_data *data)
+static int mxt_process_messages_t44(struct mxt_data *data)
{
struct device *dev = &data->client->dev;
int ret;
@@ -1499,7 +1499,7 @@ static irqreturn_t mxt_process_messages_t44(struct mxt_data *data)
data->T5_msg_size + 1, data->msg_buf);
if (ret) {
dev_err(dev, "Failed to read T44 and T5 (%d)\n", ret);
- return IRQ_NONE;
+ return ret;
}
T5_msg_count = data->msg_buf[0];
@@ -1509,7 +1509,7 @@ static irqreturn_t mxt_process_messages_t44(struct mxt_data *data)
* Mode 0. It results in unnecessary I2C operations but it is benign.
*/
if (!T5_msg_count)
- return IRQ_NONE;
+ return processed_valid;
if (T5_msg_count > data->max_reportid) {
dev_warn(dev, "T44 count %d exceeded max report id\n",
@@ -1521,12 +1521,14 @@ static irqreturn_t mxt_process_messages_t44(struct mxt_data *data)
ret = mxt_proc_message(data, data->msg_buf + 1);
if (ret < 0) {
dev_warn(dev, "Unexpected invalid message\n");
- return IRQ_NONE;
+ return ret;
}
total_pending = T5_msg_count - 1;
- if (!total_pending)
+ if (!total_pending) {
+ processed_valid = 1;
goto end;
+ }
/* Process remaining messages if necessary */
T5_msg_count = mxt_max_msg_read_count(data, total_pending);
@@ -1550,7 +1552,7 @@ static irqreturn_t mxt_process_messages_t44(struct mxt_data *data)
data->update_input = false;
}
- return IRQ_HANDLED;
+ return processed_valid;
}
static int mxt_process_messages_until_invalid(struct mxt_data *data)
@@ -1580,7 +1582,7 @@ static int mxt_process_messages_until_invalid(struct mxt_data *data)
return -EBUSY;
}
-static irqreturn_t mxt_process_messages(struct mxt_data *data)
+static int mxt_process_messages(struct mxt_data *data)
{
int total_handled, num_handled;
u8 count = data->last_message_count;
@@ -1591,7 +1593,7 @@ static irqreturn_t mxt_process_messages(struct mxt_data *data)
/* include final invalid message */
total_handled = mxt_read_and_process_messages(data, count + 1);
if (total_handled < 0)
- return IRQ_NONE;
+ return total_handled;
/* if there were invalid messages, then we are done */
else if (total_handled <= count)
goto update_count;
@@ -1600,7 +1602,7 @@ static irqreturn_t mxt_process_messages(struct mxt_data *data)
do {
num_handled = mxt_read_and_process_messages(data, 2);
if (num_handled < 0)
- return IRQ_NONE;
+ return num_handled;
total_handled += num_handled;
@@ -1616,12 +1618,13 @@ static irqreturn_t mxt_process_messages(struct mxt_data *data)
data->update_input = false;
}
- return IRQ_HANDLED;
+ return total_handled;
}
static irqreturn_t mxt_interrupt(int irq, void *dev_id)
{
struct mxt_data *data = dev_id;
+ int ret;
if (data->in_bootloader) {
complete(&data->chg_completion);
@@ -1629,17 +1632,22 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)
if (data->flash && &data->flash->work)
cancel_delayed_work_sync(&data->flash->work);
- return IRQ_RETVAL(mxt_check_bootloader(data));
+ ret = mxt_check_bootloader(data);
+ return IRQ_RETVAL(ret);
}
if (!data->object_table)
return IRQ_HANDLED;
- if (data->T44_address) {
- return mxt_process_messages_t44(data);
- } else {
- return mxt_process_messages(data);
- }
+ if (data->T44_address)
+ ret = mxt_process_messages_t44(data);
+ else
+ ret = mxt_process_messages(data);
+
+ if (ret <= 0)
+ return IRQ_NONE;
+ else
+ return IRQ_HANDLED;
}
static int mxt_t6_command(struct mxt_data *data, u16 cmd_offset,
@@ -1774,8 +1782,11 @@ static int mxt_acquire_irq(struct mxt_data *data)
}
if (data->object_table && data->use_retrigen_workaround) {
- error = mxt_process_messages_until_invalid(data);
- if (error)
+ if (data->T44_address)
+ error = mxt_process_messages_t44(data);
+ else
+ error = mxt_process_messages_until_invalid(data);
+ if (error < 0)
return error;
}
@@ -4029,8 +4040,12 @@ static int mxt_start(struct mxt_data *data)
* Discard any touch messages still in message buffer
* from before chip went to sleep
*/
- ret = mxt_process_messages_until_invalid(data);
- if (ret)
+
+ if (data->T44_address)
+ ret = mxt_process_messages_t44(data);
+ else
+ ret = mxt_process_messages_until_invalid(data);
+ if (ret < 0)
break;
ret = mxt_set_t7_power_cfg(data, MXT_POWER_CFG_RUN);
--
2.19.2
^ permalink raw reply related
* [PATCH v2 45/49] Input: atmel_mxt_ts: use gpiod_set_value_cansleep for reset pin
From: Jiada Wang @ 2019-08-27 6:31 UTC (permalink / raw)
To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
From: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
In case of remote display, touch controller will be also remote.
In such cases, the reset pin of the touch controller will be
controlled through bridging ICs like Deserilizer and Serializer.
Therefore accessing the gpio pins require transactions with the
external IC. Using the function gpiod_set_value will print a
warning like below
WARNING: CPU: 0 PID: 576 at drivers/gpio/gpiolib.c:1441 gpiod_set_value+0x34/0x60()
CPU: 0 PID: 576 Comm: modprobe Not tainted 3.14.79-08377-g84ea22f-dirty #4
Backtrace:
[<80011c58>] (dump_backtrace) from [<80011e60>] (show_stack+0x18/0x1c)
[<80011e48>] (show_stack) from [<8052d7ac>] (dump_stack+0x7c/0x9c)
[<8052d730>] (dump_stack) from [<800241bc>] (warn_slowpath_common+0x74/0x9c)
[<80024148>] (warn_slowpath_common) from [<80024288>] (warn_slowpath_null+0x24/0x2c)
[<80024264>] (warn_slowpath_null) from [<8029e070>] (gpiod_set_value+0x34/0x60)
[<8029e03c>] (gpiod_set_value) from [<7f492e98>] (mxt_probe+0x1e0/0x718 [atmel_mxt_ts])
[<7f492cb8>] (mxt_probe [atmel_mxt_ts]) from [<803c4d34>] (i2c_device_probe+0xcc/0xec)
[<803c4c68>] (i2c_device_probe) from [<803252a0>] (driver_probe_device+0xc0/0x200)
Signed-off-by: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@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 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 58e54eb45cf0..1187e21a67e4 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2487,7 +2487,7 @@ static void mxt_regulator_enable(struct mxt_data *data)
if (!data->reg_vdd || !data->reg_avdd)
return;
- gpiod_set_value(data->reset_gpio, 0);
+ gpiod_set_value_cansleep(data->reset_gpio, 0);
error = regulator_enable(data->reg_vdd);
if (error)
@@ -2505,7 +2505,7 @@ static void mxt_regulator_enable(struct mxt_data *data)
* voltage
*/
msleep(MXT_REGULATOR_DELAY);
- gpiod_set_value(data->reset_gpio, 1);
+ gpiod_set_value_cansleep(data->reset_gpio, 1);
msleep(MXT_CHG_DELAY);
retry_wait:
@@ -4311,7 +4311,7 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
disable_irq(data->irq);
} else if (data->reset_gpio) {
msleep(MXT_RESET_GPIO_TIME);
- gpiod_set_value(data->reset_gpio, 1);
+ gpiod_set_value_cansleep(data->reset_gpio, 1);
msleep(MXT_RESET_INVALID_CHG);
} else {
dev_dbg(&client->dev,
--
2.19.2
^ permalink raw reply related
* [PATCH v2 46/49] input: touchscreen: atmel_mxt_ts: Added sysfs entry for touchscreen status
From: Jiada Wang @ 2019-08-27 6:31 UTC (permalink / raw)
To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190827063201.21048-1-jiada_wang@mentor.com>
From: Naveen Chakka <Naveen.Chakka@in.bosch.com>
To know the current communication status of the touch controller during
runtime, sysfs interface is added
sysfs interface: /sys/class/i2c-dev/i2c-*/device/*/touch_dev_stat
Executing the above sysfs interface provides two output values
1)Status of the touch device
value 0 represents device is inactive
value 1 represents device is active
2)Error counter
value represents the number of times device in inactive since last read
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 | 109 +++++++++++++++++++++--
1 file changed, 102 insertions(+), 7 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 1187e21a67e4..5a112dfe30e4 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -25,6 +25,7 @@
#include <linux/property.h>
#include <linux/slab.h>
#include <linux/gpio/consumer.h>
+#include <linux/timer.h>
#include <asm/unaligned.h>
#include <linux/regulator/consumer.h>
#include <linux/workqueue.h>
@@ -222,6 +223,7 @@ enum t100_type {
#define MXT_CHG_DELAY 100 /* msec */
#define MXT_POWERON_DELAY 150 /* msec */
#define MXT_BOOTLOADER_WAIT 36E5 /* 1 minute */
+#define MXT_WATCHDOG_TIMEOUT 1000 /* msec */
/* Command to unlock bootloader */
#define MXT_UNLOCK_CMD_MSB 0xaa
@@ -317,6 +319,12 @@ struct mxt_flash {
struct delayed_work work;
};
+struct mxt_statusinfo {
+ bool dev_status;
+ bool intp_triggered;
+ u32 error_count;
+};
+
/* Each client has this additional data */
struct mxt_data {
struct i2c_client *client;
@@ -372,6 +380,9 @@ struct mxt_data {
const char *pcfg_name;
const char *input_name;
struct mxt_flash *flash;
+ struct work_struct watchdog_work;
+ struct timer_list watchdog_timer;
+ struct mxt_statusinfo mxt_status;
/* Cached parameters from object table */
u16 T5_address;
@@ -1621,11 +1632,30 @@ static int mxt_process_messages(struct mxt_data *data)
return total_handled;
}
+static void mxt_start_wd_timer(struct mxt_data *data)
+{
+ mod_timer(&data->watchdog_timer, jiffies +
+ msecs_to_jiffies(MXT_WATCHDOG_TIMEOUT));
+}
+
+static void mxt_stop_wd_timer(struct mxt_data *data)
+{
+ /*
+ * Ensure we wait until the watchdog timer
+ * running on a different CPU finishes
+ */
+ del_timer_sync(&data->watchdog_timer);
+ cancel_work_sync(&data->watchdog_work);
+ del_timer_sync(&data->watchdog_timer);
+}
+
static irqreturn_t mxt_interrupt(int irq, void *dev_id)
{
struct mxt_data *data = dev_id;
int ret;
+ data->mxt_status.intp_triggered = true;
+
if (data->in_bootloader) {
complete(&data->chg_completion);
@@ -1633,21 +1663,25 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)
cancel_delayed_work_sync(&data->flash->work);
ret = mxt_check_bootloader(data);
- return IRQ_RETVAL(ret);
+ ret = IRQ_RETVAL(ret);
+ goto exit;
}
- if (!data->object_table)
- return IRQ_HANDLED;
+ if (!data->object_table) {
+ ret = IRQ_HANDLED;
+ goto exit;
+ }
if (data->T44_address)
ret = mxt_process_messages_t44(data);
else
ret = mxt_process_messages(data);
- if (ret <= 0)
- return IRQ_NONE;
- else
- return IRQ_HANDLED;
+ ret = (ret <= 0) ? IRQ_NONE : IRQ_HANDLED;
+
+exit:
+ data->mxt_status.intp_triggered = false;
+ return ret;
}
static int mxt_t6_command(struct mxt_data *data, u16 cmd_offset,
@@ -2967,6 +3001,36 @@ static int mxt_bootloader_status(struct mxt_data *data)
return 0;
}
+static void mxt_watchdog_timer(struct timer_list *t)
+{
+ struct mxt_data *data = from_timer(data, t, watchdog_timer);
+
+ if (!work_pending(&data->watchdog_work)) {
+ if (!data->mxt_status.intp_triggered)
+ schedule_work(&data->watchdog_work);
+ }
+
+ mxt_start_wd_timer(data);
+}
+
+static void mxt_watchdog_work(struct work_struct *work)
+{
+ struct mxt_data *data =
+ container_of(work, struct mxt_data, watchdog_work);
+ u16 info_buf;
+ int ret = 0;
+ u8 size = 2;
+
+ ret = __mxt_read_reg(data->client, 0, size, &info_buf);
+
+ if (ret) {
+ data->mxt_status.error_count++;
+ data->mxt_status.dev_status = false;
+ } else {
+ data->mxt_status.dev_status = true;
+ }
+}
+
static int mxt_initialize(struct mxt_data *data)
{
struct i2c_client *client = data->client;
@@ -3944,6 +4008,22 @@ static const struct attribute_group mxt_fw_attr_group = {
.attrs = mxt_fw_attrs,
};
+static ssize_t mxt_touch_device_status(struct device *dev, struct
+ device_attribute *attr, char *buf)
+{
+ struct mxt_data *data = dev_get_drvdata(dev);
+ int ret = 0;
+
+ if (data->mxt_status.dev_status)
+ data->mxt_status.error_count = 0;
+
+ ret = snprintf(buf, PAGE_SIZE, "%d %d\n", data->mxt_status.dev_status,
+ data->mxt_status.error_count);
+ /* clear the error counter once it is read */
+ data->mxt_status.error_count = 0;
+ 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);
@@ -3955,6 +4035,7 @@ static DEVICE_ATTR(debug_v2_enable, S_IWUSR | S_IRUSR, NULL,
mxt_debug_v2_enable_store);
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 struct attribute *mxt_attrs[] = {
&dev_attr_fw_version.attr,
@@ -3966,6 +4047,7 @@ static struct attribute *mxt_attrs[] = {
&dev_attr_debug_v2_enable.attr,
&dev_attr_debug_notify.attr,
&dev_attr_t25.attr,
+ &dev_attr_touch_dev_stat.attr,
NULL
};
@@ -4319,6 +4401,13 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
msleep(MXT_RESET_TIME);
}
+ INIT_WORK(&data->watchdog_work, mxt_watchdog_work);
+
+ /* setup watchdog timer */
+ timer_setup(&data->watchdog_timer, mxt_watchdog_timer, 0);
+
+ mxt_start_wd_timer(data);
+
error = mxt_initialize(data);
if (error)
goto err_free_object;
@@ -4333,8 +4422,11 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
return 0;
err_free_object:
+ cancel_work_sync(&data->watchdog_work);
+ mxt_stop_wd_timer(data);
mxt_free_input_device(data);
mxt_free_object_table(data);
+ del_timer(&data->watchdog_timer);
if (data->reset_gpio) {
sysfs_remove_link(&client->dev.kobj, "reset");
gpiod_unexport(data->reset_gpio);
@@ -4357,6 +4449,9 @@ static int mxt_remove(struct i2c_client *client)
mxt_free_input_device(data);
mxt_free_object_table(data);
+ cancel_work_sync(&data->watchdog_work);
+ mxt_stop_wd_timer(data);
+
return 0;
}
--
2.19.2
^ permalink raw reply related
* [PATCH v2 47/49] input: atmel_mxt_ts: added sysfs interface to update atmel T38 data
From: Jiada Wang @ 2019-08-27 6:31 UTC (permalink / raw)
To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190827063201.21048-1-jiada_wang@mentor.com>
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 | 102 +++++++++++++++++++++++
1 file changed, 102 insertions(+)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 5a112dfe30e4..63ff8a211a90 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -4024,6 +4024,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);
@@ -4036,6 +4136,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,
@@ -4048,6 +4149,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 v2 48/49] Input: atmel_mxt_ts: Implement synchronization during various operation
From: Jiada Wang @ 2019-08-27 6:32 UTC (permalink / raw)
To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190827063201.21048-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 63ff8a211a90..a461220cd336 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;
@@ -426,11 +441,15 @@ struct mxt_data {
/* Indicates whether device is in suspend */
bool suspended;
- /* Indicates whether device is updating configuration */
- bool updating_config;
+ struct mutex lock;
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_vb2_buffer {
@@ -1654,6 +1673,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) {
@@ -1681,6 +1701,8 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)
exit:
data->mxt_status.intp_triggered = false;
+ mutex_unlock(&data->lock);
+
return ret;
}
@@ -2261,6 +2283,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;
@@ -2290,6 +2314,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,
@@ -2971,8 +2997,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)
@@ -3085,6 +3118,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,
@@ -3864,30 +3906,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,
@@ -3895,14 +3965,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);
@@ -3910,8 +4004,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) {
@@ -3927,15 +4019,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;
}
@@ -4199,8 +4284,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:
@@ -4244,8 +4338,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;
}
@@ -4254,8 +4352,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:
@@ -4285,8 +4394,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)
@@ -4441,12 +4557,15 @@ 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);
+
data->client = client;
i2c_set_clientdata(client, data);
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) ?
@@ -4540,6 +4659,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) {
@@ -4596,6 +4727,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 v2 49/49] Input: atmel_mxt_ts - Fix compilation warning
From: Jiada Wang @ 2019-08-27 6:32 UTC (permalink / raw)
To: nick, dmitry.torokhov; +Cc: linux-input, linux-kernel, jiada_wang, george_davis
In-Reply-To: <20190827063201.21048-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 a461220cd336..115c94d3f0d4 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2046,7 +2046,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] input: keyboard: snvs_pwrkey: Send press and release event for i.MX6 S,DL and Q
From: robin @ 2019-08-27 7:55 UTC (permalink / raw)
To: Robin Gong
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, dl-linux-imx,
Dmitry Torokhov, devicetree, linux-arm-kernel, linux-kernel,
linux-input, Adam Ford
In-Reply-To: <VE1PR04MB6638754916357F551502102589A00@VE1PR04MB6638.eurprd04.prod.outlook.com>
On 2019-08-27 08:17, Robin Gong wrote:
> On Fri, Aug 23, 2019 at 02:30:02PM +0200, Robin van der Gracht wrote:>
>> The older generation i.MX6 processors send a powerdown request
>> interrupt
>> if the powerkey is released before a hard shutdown (5 second press).
>> This
>> should allow software to bring down the SoC safely.
>>
>> For this driver to work as a regular powerkey with the older SoCs, we
>> need to
>> send a keypress AND release when we get the powerdown request
>> interrupt.
> Please clarify here more clearly that because there is NO press
> interrupt triggered
> but only release interrupt on elder i.mx6 processors and that HW issue
> fixed from
> i.mx6sx.
ACK
>>
>> Signed-off-by: Robin van der Gracht <robin@protonic.nl>
>> ---
>> arch/arm/boot/dts/imx6qdl.dtsi | 2 +-
>> arch/arm/boot/dts/imx6sll.dtsi | 2 +-
>> arch/arm/boot/dts/imx6sx.dtsi | 2 +-
>> arch/arm/boot/dts/imx6ul.dtsi | 2 +-
>> arch/arm/boot/dts/imx7s.dtsi | 2 +-
> As Shawn talked, please keep the original "fsl,sec-v4.0-pwrkey", just
> add
> 'imx6qdl-snvs-pwrkey' for elder i.mx6 processor i.mx6q/dl/sl, thus no
> need
> to touch other newer processor's dts.
ACK
>
>>
>> static void imx_imx_snvs_check_for_events(struct timer_list *t) @@
>> -67,13
>> +85,23 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void
>> *dev_id) {
>> struct platform_device *pdev = dev_id;
>> struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev);
>> + struct input_dev *input = pdata->input;
>> u32 lp_status;
>>
>> - pm_wakeup_event(pdata->input->dev.parent, 0);
>> + pm_wakeup_event(input->dev.parent, 0);
>>
>> regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
>> - if (lp_status & SNVS_LPSR_SPO)
>> - mod_timer(&pdata->check_timer, jiffies +
>> msecs_to_jiffies(DEBOUNCE_TIME));
>> + if (lp_status & SNVS_LPSR_SPO) {
>> + if (pdata->hwtype == IMX6QDL_SNVS) {
>> + input_report_key(input, pdata->keycode, 1);
>> + input_report_key(input, pdata->keycode, 0);
>> + input_sync(input);
>> + pm_relax(input->dev.parent);
> Could you move the above input event report steps into
> imx_imx_snvs_check_for_events()
> as before? That make code better to understand and less operation in
> ISR.
I placed it here to avoid the unnessesairy debounce delay (since thats
already
implemented in hardware).
I do agree with your arguments so I'll move emitting the events to
imx_imx_snvs_check_for_events().
Is it ok if I keep the conditional, but instead of emitting the events,
schedule imx_imx_snvs_check_for_events() immidiatly to avoid the
debounce,
or should I choose clarity over the 30 ms delay?
>> + } else {
>> + mod_timer(&pdata->check_timer,
>> + jiffies + msecs_to_jiffies(DEBOUNCE_TIME));
>> + }
>> + }
>>
>> /* clear SPO status */
>> regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO); @@
>> -88,11 +116,24 @@ static void imx_snvs_pwrkey_act(void *pdata)
>> del_timer_sync(&pd->check_timer);
>> }
>>
>> +static const struct of_device_id imx_snvs_pwrkey_ids[] = {
>> + {
>> + .compatible = "fsl,imx6sx-sec-v4.0-pwrkey",
>> + .data = &imx_snvs_devtype[IMX6SX_SNVS],
>> + }, {
>> + .compatible = "fsl,imx6qdl-sec-v4.0-pwrkey",
>> + .data = &imx_snvs_devtype[IMX6QDL_SNVS],
> No ' IMX6QDL_SNVS ' defined in your patch or am I missing?
I added an enum 'imx_snvs_hwtype' that defines both IMX6SX_SNVS and
IMX6QDL_SNVS.
>> + },
>> + { /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids);
>> --
>> 2.20.1
^ permalink raw reply
* [PATCH v2 1/2] input: keyboard: snvs_pwrkey: Send key events for i.MX6 S, DL and Q
From: Robin van der Gracht @ 2019-08-27 12:32 UTC (permalink / raw)
To: Robin Gong
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, dl-linux-imx,
Dmitry Torokhov, devicetree @ vger . kernel . org,
linux-arm-kernel @ lists . infradead . org,
linux-kernel @ vger . kernel . org,
linux-input @ vger . kernel . org, Adam Ford,
Robin van der Gracht
The first generation i.MX6 processors does not send an interrupt when the
power key is pressed. It sends a power down request interrupt if the key is
released before a hard shutdown (5 second press). This should allow
software to bring down the SoC safely.
For this driver to work as a regular power key with the older SoCs, we need
to send a keypress AND release when we get the power down request irq.
Signed-off-by: Robin van der Gracht <robin@protonic.nl>
---
.../devicetree/bindings/crypto/fsl-sec4.txt | 16 ++++--
drivers/input/keyboard/Kconfig | 2 +-
drivers/input/keyboard/snvs_pwrkey.c | 52 ++++++++++++++++---
3 files changed, 57 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
index 2fe245ca816a..e4fbb9797082 100644
--- a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
+++ b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
@@ -420,14 +420,22 @@ EXAMPLE
=====================================================================
System ON/OFF key driver
- The snvs-pwrkey is designed to enable POWER key function which controlled
- by SNVS ONOFF, the driver can report the status of POWER key and wakeup
- system if pressed after system suspend.
+ The snvs-pwrkey is designed to enable POWER key function which is controlled
+ by SNVS ONOFF. It can wakeup the system if pressed after system suspend.
+
+ There are two generations of SVNS pwrkey hardware. The first generation is
+ included in i.MX6 Solo, DualLite and Quad processors. The second generation
+ is included in i.MX6 SoloX and newer SoCs.
+
+ Second generation SNVS can detect and report the status of POWER key, but the
+ first generation can only detect a key release and so emits an instantaneous
+ press and release event when the key is released.
- compatible:
Usage: required
Value type: <string>
- Definition: Mush include "fsl,sec-v4.0-pwrkey".
+ Definition: Must include "fsl,sec-v4.0-pwrkey" for i.MX6 SoloX and newer
+ or "fsl,imx6qdl-snvs-pwrkey" for older SoCs.
- interrupts:
Usage: required
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 7c4f19dab34f..937e58da5ce1 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -436,7 +436,7 @@ config KEYBOARD_SNVS_PWRKEY
depends on OF
help
This is the snvs powerkey driver for the Freescale i.MX application
- processors that are newer than i.MX6 SX.
+ processors.
To compile this driver as a module, choose M here; the
module will be called snvs_pwrkey.
diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
index 5342d8d45f81..d71c44733103 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -29,6 +29,11 @@
#define DEBOUNCE_TIME 30
#define REPEAT_INTERVAL 60
+enum imx_snvs_hwtype {
+ IMX6SX_SNVS, /* i.MX6 SoloX and newer */
+ IMX6QDL_SNVS, /* i.MX6 Solo, DualLite and Quad */
+};
+
struct pwrkey_drv_data {
struct regmap *snvs;
int irq;
@@ -37,14 +42,41 @@ struct pwrkey_drv_data {
int wakeup;
struct timer_list check_timer;
struct input_dev *input;
+ enum imx_snvs_hwtype hwtype;
};
+static const struct of_device_id imx_snvs_pwrkey_ids[] = {
+ {
+ .compatible = "fsl,sec-v4.0-pwrkey",
+ .data = (const void *)IMX6SX_SNVS,
+ },
+ {
+ .compatible = "fsl,imx6qdl-snvs-pwrkey",
+ .data = (const void *)IMX6QDL_SNVS,
+ },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids);
+
static void imx_imx_snvs_check_for_events(struct timer_list *t)
{
struct pwrkey_drv_data *pdata = from_timer(pdata, t, check_timer);
struct input_dev *input = pdata->input;
u32 state;
+ if (pdata->hwtype == IMX6QDL_SNVS) {
+ /*
+ * The first generation i.MX6 SoCs only sends an interrupt on
+ * button release. To mimic power-key usage, we'll prepend a
+ * press event.
+ */
+ input_report_key(input, pdata->keycode, 1);
+ input_report_key(input, pdata->keycode, 0);
+ input_sync(input);
+ pm_relax(input->dev.parent);
+ return;
+ }
+
regmap_read(pdata->snvs, SNVS_HPSR_REG, &state);
state = state & SNVS_HPSR_BTN ? 1 : 0;
@@ -67,13 +99,17 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id)
{
struct platform_device *pdev = dev_id;
struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev);
+ unsigned long expire = jiffies;
u32 lp_status;
pm_wakeup_event(pdata->input->dev.parent, 0);
regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
- if (lp_status & SNVS_LPSR_SPO)
- mod_timer(&pdata->check_timer, jiffies + msecs_to_jiffies(DEBOUNCE_TIME));
+ if (lp_status & SNVS_LPSR_SPO) {
+ if (pdata->hwtype == IMX6SX_SNVS)
+ expire += msecs_to_jiffies(DEBOUNCE_TIME);
+ mod_timer(&pdata->check_timer, expire);
+ }
/* clear SPO status */
regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO);
@@ -93,6 +129,7 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
struct pwrkey_drv_data *pdata = NULL;
struct input_dev *input = NULL;
struct device_node *np;
+ const struct of_device_id *match;
int error;
/* Get SNVS register Page */
@@ -100,6 +137,10 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
if (!np)
return -ENODEV;
+ match = of_match_node(imx_snvs_pwrkey_ids, np);
+ if (!match)
+ return -ENODEV;
+
pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
return -ENOMEM;
@@ -115,6 +156,7 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n");
}
+ pdata->hwtype = (enum imx_snvs_hwtype)match->data;
pdata->wakeup = of_property_read_bool(np, "wakeup-source");
pdata->irq = platform_get_irq(pdev, 0);
@@ -175,12 +217,6 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
return 0;
}
-static const struct of_device_id imx_snvs_pwrkey_ids[] = {
- { .compatible = "fsl,sec-v4.0-pwrkey" },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids);
-
static struct platform_driver imx_snvs_pwrkey_driver = {
.driver = {
.name = "snvs_pwrkey",
--
2.20.1
^ permalink raw reply related
* [PATCH v2 2/2] arm: dts: imx6qdl: snvs-pwrkey: Change compatible string
From: Robin van der Gracht @ 2019-08-27 12:32 UTC (permalink / raw)
To: Robin Gong
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, dl-linux-imx,
Dmitry Torokhov, devicetree @ vger . kernel . org,
linux-arm-kernel @ lists . infradead . org,
linux-kernel @ vger . kernel . org,
linux-input @ vger . kernel . org, Adam Ford,
Robin van der Gracht
In-Reply-To: <20190827123216.32728-1-robin@protonic.nl>
The older imx6 SoCs do not send a power key press interrupt, instead it
sends a power down request interrupt when the key is released between
750ms and 5 seconds. The driver uses a different compatible string to ID
the older SoCs.
Signed-off-by: Robin van der Gracht <robin@protonic.nl>
---
arch/arm/boot/dts/imx6qdl.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index b3a77bcf00d5..91b97816881c 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -836,7 +836,7 @@
};
snvs_pwrkey: snvs-powerkey {
- compatible = "fsl,sec-v4.0-pwrkey";
+ compatible = "fsl,imx6qdl-snvs-pwrkey";
regmap = <&snvs>;
interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
linux,keycode = <KEY_POWER>;
--
2.20.1
^ permalink raw reply related
* [PATCH] Input: modify quirks of i2c-hid driver for weida's devices
From: hn.chen @ 2019-08-27 12:47 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, dmitry.torokhov, hn.chen
From: HungNien Chen <hn.chen@weidahitech.com>
This 'SET_PWR_WAKEUP_DEV' quirk only works for weida's devices with pid
0xC300 & 0xC301. Some weida's devices with other pids also need this quirk
now. Use 'HID_ANY_ID' instead of 0xC300 to make all of weida's devices can be
fixed on the power on issue. This modification should be safe since devices
without power on issue will send the power on command only once.
Signed-off-by: HungNien Chen <hn.chen@weidahitech.com>
---
drivers/hid/i2c-hid/i2c-hid-core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 90164fe..2a7c6e3 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -169,9 +169,7 @@ struct i2c_hid {
__u16 idProduct;
__u32 quirks;
} i2c_hid_quirks[] = {
- { USB_VENDOR_ID_WEIDA, USB_DEVICE_ID_WEIDA_8752,
- I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
- { USB_VENDOR_ID_WEIDA, USB_DEVICE_ID_WEIDA_8755,
+ { USB_VENDOR_ID_WEIDA, HID_ANY_ID,
I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
I2C_HID_QUIRK_NO_IRQ_AFTER_RESET |
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] HID: apple: Fix stuck function keys when using FN
From: Benjamin Tissoires @ 2019-08-27 16:57 UTC (permalink / raw)
To: Benjamin Tissoires, João Moreno
Cc: Jiri Kosina, open list:HID CORE LAYER, lkml
In-Reply-To: <CAN+gG=H6O202SYXGbTG5rTMUt_X5dZyi02YFUquPaqL=FGHXwQ@mail.gmail.com>
Hi João,
On 8/12/19 6:57 PM, Benjamin Tissoires wrote:
> Hi João,
>
> On Thu, Aug 8, 2019 at 10:35 PM João Moreno <mail@joaomoreno.com> wrote:
>>
>> Hi Benjamin,
>>
>> On Mon, 8 Jul 2019 at 22:35, João Moreno <mail@joaomoreno.com> wrote:
>>>
>>> Hi Benjamin,
>>>
>>> No worries, also pretty busy over here. Didn't mean to press.
>>>
>>> On Mon, 1 Jul 2019 at 10:32, Benjamin Tissoires
>>> <benjamin.tissoires@redhat.com> wrote:
>>>>
>>>> Hi João,
>>>>
>>>> On Sun, Jun 30, 2019 at 10:15 PM João Moreno <mail@joaomoreno.com> wrote:
>>>>>
>>>>> Hi Jiri & Benjamin,
>>>>>
>>>>> Let me know if you need something else to get this patch moving forward. This
>>>>> fixes an issue I hit daily, it would be great to get it fixed.
>>>>
>>>> Sorry for the delay, I am very busy with internal corporate stuff, and
>>>> I tried setting up a new CI system at home, and instead of spending a
>>>> couple of ours, I am down to 2 weeks of hard work, without possibility
>>>> to switch to the new right now :(
>>>> Anyway.
>>>>
>>>>>
>>>>> Thanks.
>>>>>
>>>>> On Mon, 10 Jun 2019 at 23:31, Joao Moreno <mail@joaomoreno.com> wrote:
>>>>>>
>>>>>> This fixes an issue in which key down events for function keys would be
>>>>>> repeatedly emitted even after the user has raised the physical key. For
>>>>>> example, the driver fails to emit the F5 key up event when going through
>>>>>> the following steps:
>>>>>> - fnmode=1: hold FN, hold F5, release FN, release F5
>>>>>> - fnmode=2: hold F5, hold FN, release F5, release FN
>>>>
>>>> Ouch :/
>>>>
>>>
>>> Right?!
>>>
>>>>>>
>>>>>> The repeated F5 key down events can be easily verified using xev.
>>>>>>
>>>>>> Signed-off-by: Joao Moreno <mail@joaomoreno.com>
>>>>>> ---
>>>>>> drivers/hid/hid-apple.c | 21 +++++++++++----------
>>>>>> 1 file changed, 11 insertions(+), 10 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
>>>>>> index 1cb41992aaa1..81867a6fa047 100644
>>>>>> --- a/drivers/hid/hid-apple.c
>>>>>> +++ b/drivers/hid/hid-apple.c
>>>>>> @@ -205,20 +205,21 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
>>>>>> trans = apple_find_translation (table, usage->code);
>>>>>>
>>>>>> if (trans) {
>>>>>> - if (test_bit(usage->code, asc->pressed_fn))
>>>>>> - do_translate = 1;
>>>>>> - else if (trans->flags & APPLE_FLAG_FKEY)
>>>>>> - do_translate = (fnmode == 2 && asc->fn_on) ||
>>>>>> - (fnmode == 1 && !asc->fn_on);
>>>>>> + int fn_on = value ? asc->fn_on :
>>>>>> + test_bit(usage->code, asc->pressed_fn);
>>>>>> +
>>>>>> + if (!value)
>>>>>> + clear_bit(usage->code, asc->pressed_fn);
>>>>>> + else if (asc->fn_on)
>>>>>> + set_bit(usage->code, asc->pressed_fn);
>>>>
>>>> I have the feeling that this is not the correct fix here.
>>>>
>>>> I might be wrong, but the following sequence might also mess up the
>>>> driver state, depending on how the reports are emitted:
>>>> - hold FN, hold F4, hold F5, release F4, release FN, release F5
>>>>
>>>
>>> I believe this should be fine. Following the code:
>>>
>>> - hold FN, sets asc->fn_on to true
>>> - hold F4, in the trans block fn_on will be true and we'll set the F4
>>> bit in the bitmap
>>> - hold F5, in the trans block fn_on will be true and we'll set the F5 bit
>>> - release F4, in the trans block fn_on will be true (because of the bitmap) and
>>> we'll clear the F4 bit
>>> - release FN, asc->fn_on will be false, but it doesn't matter since...
>>> - release F5, in the trans block we'll look into the bitmap (instead
>>> of asc->fn_on),
>>> so fn_on will be true and we'll clear the F5 bit
>>>
>>> I tested it in practice using my changes:
>>>
>>> Interestingly the Apple keyboard doesn't seem to emit an even for F5 when F4 is
>>> pressed, seems like a hardware limitation. But F6 does work. So, when I execute
>>> these events in that order, everything works as it should: xev reports
>>> the following:
>>>
>>> KeyPress F4
>>> KeyPress F6
>>> KeyRelease F4
>>> KeyRelease F6
>>>
>>>> The reason is that the driver only considers you have one key pressed
>>>> with the modifier, and as the code changed its state based on the last
>>>> value.
>>>>
>>>
>>> I believe the bitmap takes care of storing the FN state per key press. The
>>> trick I did was to check on the global `asc->fn_on` state only when a key
>>> is pressed, but check on the bitmap instead when it's released.
>>>
>>> Let me know what you think. Am I missing something here?
>>>
>>> Cheers,
>>> João.
>>>
>>>> IMO a better fix would:
>>>>
>>>> - keep the existing `trans` mapping lookout
>>>> - whenever a `trans` mapping gets found:
>>>> * get both translated and non-translated currently reported values
>>>> (`test_bit(keycode, input_dev->key)`)
>>>> * if one of them is set to true, then consider the keycode to be the
>>>> one of the key (no matter fn_on)
>>>> -> deal with `value` with the corrected keycode
>>>> * if the key was not pressed:
>>>> -> chose the keycode based on `fn_on` and `fnmode` states
>>>> and report the key press event
>>>>
>>>> This should remove the nasty pressed_fn state which depends on the
>>>> other pressed keys.
>>>>
>>>> Cheers,
>>>> Benjamin
>>>>
>>>>>> +
>>>>>> + if (trans->flags & APPLE_FLAG_FKEY)
>>>>>> + do_translate = (fnmode == 2 && fn_on) ||
>>>>>> + (fnmode == 1 && !fn_on);
>>>>>> else
>>>>>> do_translate = asc->fn_on;
>>>>>>
>>>>>> if (do_translate) {
>>>>>> - if (value)
>>>>>> - set_bit(usage->code, asc->pressed_fn);
>>>>>> - else
>>>>>> - clear_bit(usage->code, asc->pressed_fn);
>>>>>> -
>>>>>> input_event(input, usage->type, trans->to,
>>>>>> value);
>>>>>>
>>>>>> --
>>>>>> 2.19.1
>>>>>>
>>
>> Gave this another look and I still haven't found any issues, let me
>> know if you still
>> think I'm missing something. Thanks!
>>
>
> OK, I am tempted to take the patch, but I'll need to add this as a
> regression test in my test-suite. This is too complex that it can
> easily break next time someone looks at it.
>
> Can you send me the report descriptors of the device using
> hid-recorder from hid-tools[0].
> Ideally, if you can put the faulty sequence in the logs, that would
> help writing down the use case.
>
I finally managed to get the regression tests up in
https://gitlab.freedesktop.org/libevdev/hid-tools/merge_requests/52
This allowed me to better understand the code, and yes, for the F-keys,
I could not find a faulty situation with your patch.
However, the same problem is happening with the arrow keys, as arrow up
is translated to page up.
We *could* adapt your patch to solve this, but I find it really hard to understand.
How about the following diff:
---
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 81df62f48c4c..ef916c0f3c0b 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -54,7 +54,6 @@ MODULE_PARM_DESC(swap_opt_cmd, "Swap the Option (\"Alt\") and Command (\"Flag\")
struct apple_sc {
unsigned long quirks;
unsigned int fn_on;
- DECLARE_BITMAP(pressed_fn, KEY_CNT);
DECLARE_BITMAP(pressed_numlock, KEY_CNT);
};
@@ -181,6 +180,8 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
{
struct apple_sc *asc = hid_get_drvdata(hid);
const struct apple_key_translation *trans, *table;
+ bool do_translate;
+ u16 code = 0;
if (usage->code == KEY_FN) {
asc->fn_on = !!value;
@@ -189,8 +190,6 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
}
if (fnmode) {
- int do_translate;
-
if (hid->product >= USB_DEVICE_ID_APPLE_WELLSPRING4_ANSI &&
hid->product <= USB_DEVICE_ID_APPLE_WELLSPRING4A_JIS)
table = macbookair_fn_keys;
@@ -202,25 +201,33 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
trans = apple_find_translation (table, usage->code);
if (trans) {
- if (test_bit(usage->code, asc->pressed_fn))
- do_translate = 1;
- else if (trans->flags & APPLE_FLAG_FKEY)
- do_translate = (fnmode == 2 && asc->fn_on) ||
- (fnmode == 1 && !asc->fn_on);
- else
- do_translate = asc->fn_on;
-
- if (do_translate) {
- if (value)
- set_bit(usage->code, asc->pressed_fn);
- else
- clear_bit(usage->code, asc->pressed_fn);
-
- input_event(input, usage->type, trans->to,
- value);
-
- return 1;
+ if (test_bit(trans->from, input->key))
+ code = trans->from;
+ if (test_bit(trans->to, input->key))
+ code = trans->to;
+
+ if (!code) {
+ if (trans->flags & APPLE_FLAG_FKEY) {
+ switch (fnmode) {
+ case 1:
+ do_translate = !asc->fn_on;
+ break;
+ case 2:
+ do_translate = asc->fn_on;
+ break;
+ default:
+ /* should never happen */
+ do_translate = false;
+ }
+ } else {
+ do_translate = asc->fn_on;
+ }
+
+ code = do_translate ? trans->to : trans->from;
}
+
+ input_event(input, usage->type, code, value);
+ return 1;
}
if (asc->quirks & APPLE_NUMLOCK_EMULATION &&
---
This is more or less what I suggested, minus the bug fixes.
I find this easier to understand as the .pressed_fn field is not there
anymore and we just rely on the actual state of the input subsystem.
Comments?
Cheers,
Benjamin
> Cheers,
> Benjamin
>
> [0] https://gitlab.freedesktop.org/libevdev/hid-tools
>
^ permalink raw reply related
* [PATCH] Input - i8042: Enable wakeup on a stable struct device
From: Stephen Boyd @ 2019-08-27 21:34 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-kernel, linux-input, Rafael J. Wysocki
We don't know when the device will be added with device_add() in
serio_add_port() because serio_add_port() is called from a workqueue
that this driver schedules by calling serio_register_port(). The best we
can know is that the device will definitely not have been added yet when
the start callback is called on the serio device.
While it hasn't been shown to be a problem, proactively move the wakeup
enabling calls to the start hook so that we don't race with the
workqueue calling device_add(). This will avoid racy situations where
code tries to add wakeup sysfs attributes for this device from
dpm_sysfs_add() but the path in device_set_wakeup_capable() has already
done so.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/input/serio/i8042.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index e4352741c467..f32780ce22cc 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -432,6 +432,19 @@ static int i8042_start(struct serio *serio)
{
struct i8042_port *port = serio->port_data;
+ device_set_wakeup_capable(&serio->dev, true);
+
+ /*
+ * On platforms using suspend-to-idle, allow the keyboard to
+ * wake up the system from sleep by enabling keyboard wakeups
+ * by default. This is consistent with keyboard wakeup
+ * behavior on many platforms using suspend-to-RAM (ACPI S3)
+ * by default.
+ */
+ if (pm_suspend_via_s2idle() &&
+ serio == i8042_ports[I8042_KBD_PORT_NO].serio)
+ device_set_wakeup_enable(&serio->dev, true);
+
spin_lock_irq(&i8042_lock);
port->exists = true;
spin_unlock_irq(&i8042_lock);
@@ -1397,17 +1410,6 @@ static void __init i8042_register_ports(void)
(unsigned long) I8042_COMMAND_REG,
i8042_ports[i].irq);
serio_register_port(serio);
- device_set_wakeup_capable(&serio->dev, true);
-
- /*
- * On platforms using suspend-to-idle, allow the keyboard to
- * wake up the system from sleep by enabling keyboard wakeups
- * by default. This is consistent with keyboard wakeup
- * behavior on many platforms using suspend-to-RAM (ACPI S3)
- * by default.
- */
- if (pm_suspend_via_s2idle() && i == I8042_KBD_PORT_NO)
- device_set_wakeup_enable(&serio->dev, true);
}
}
--
Sent by a computer through tubes
^ permalink raw reply related
* [PATCH] Input: elants_i2c - return real value of elants_i2c_initialize()
From: Johnny.Chuang @ 2019-08-28 2:33 UTC (permalink / raw)
To: 'Dmitry Torokhov', linux-kernel, linux-input,
STRD2-莊佳霖, STRD2-蔡惠嬋
Cc: STRD2-陳崇明經理,
'梁博翔', 'jeff'
In-Reply-To: <1566958886-25756-1-git-send-email-johnny.chuang@emc.com.tw>
The return value of elants_i2c_initialize() was always 0.
It maybe register input device when initialize fail.
Signed-off-by: Johnny Chuang <johnny.chuang@emc.com.tw>
---
drivers/input/touchscreen/elants_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/elants_i2c.c
b/drivers/input/touchscreen/elants_i2c.c
index d4ad24e..9c9816f 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -571,7 +571,7 @@ static int elants_i2c_initialize(struct elants_data *ts)
if (error)
ts->iap_mode = ELAN_IAP_RECOVERY;
- return 0;
+ return error;
}
/*
--
2.7.4
^ permalink raw reply related
* Alps touchpad generates IRQ storm after S3
From: Kai-Heng Feng @ 2019-08-28 6:22 UTC (permalink / raw)
To: masaki.ota
Cc: Mario Limonciello, open list:HID CORE LAYER,
Linux Kernel Mailing List
Hi Masaki,
The Alps touchpad (044E:1220) on Dell Precision 7530 causes IRQ storm after
system suspend (S3).
Commit "HID: i2c-hid: Don't reset device upon system resume” which solves
the same issue for other vendors, cause the issue on Alps touchpad.
So I’d like to know the correct command Alps touchpad expects after system
resume.
Also Cc Mario because this could relate to BIOS.
Kai-Heng
^ permalink raw reply
* RE: Alps touchpad generates IRQ storm after S3
From: Masaki Ota @ 2019-08-28 6:35 UTC (permalink / raw)
To: Xiaojian Cao, Kai-Heng Feng
Cc: Mario Limonciello, open list:HID CORE LAYER,
Linux Kernel Mailing List, Naoki Saito
In-Reply-To: <44F93018-5F13-4932-A5AC-9D288CDF68DD@canonical.com>
Hi, Kai-Heng,
Sorry, I'm not in charge of Linux task now.
Hi, XiaoJian,
Please check the following mail.
If you have any question, please ask Kai-Heng.
Best Regards,
Masaki Ota
-----Original Message-----
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
Sent: Wednesday, August 28, 2019 3:22 PM
To: 太田 真喜 Masaki Ota <masaki.ota@alpsalpine.com>
Cc: Mario Limonciello <mario.limonciello@dell.com>; open list:HID CORE LAYER <linux-input@vger.kernel.org>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Alps touchpad generates IRQ storm after S3
Hi Masaki,
The Alps touchpad (044E:1220) on Dell Precision 7530 causes IRQ storm after system suspend (S3).
Commit "HID: i2c-hid: Don't reset device upon system resume” which solves the same issue for other vendors, cause the issue on Alps touchpad.
So I’d like to know the correct command Alps touchpad expects after system resume.
Also Cc Mario because this could relate to BIOS.
Kai-Heng
^ permalink raw reply
* RE: Alps touchpad generates IRQ storm after S3
From: Xiaojian Cao @ 2019-08-28 6:51 UTC (permalink / raw)
To: Masaki Ota, Kai-Heng Feng
Cc: Mario Limonciello, open list:HID CORE LAYER,
Linux Kernel Mailing List, Naoki Saito
In-Reply-To: <TYAPR01MB30223CB8A576C7809F6382C1ECA30@TYAPR01MB3022.jpnprd01.prod.outlook.com>
Hi Ota-san,
OK, we will look into it.
Hi Kai-Heng,
We will try to reproduce this issue first, could you please tell me the target Ubuntu version?
Best regards,
Jason
-----Original Message-----
From: 太田 真喜 Masaki Ota <masaki.ota@alpsalpine.com>
Sent: Wednesday, August 28, 2019 2:35 PM
To: 曹 曉建 Xiaojian Cao <xiaojian.cao@cn.alps.com>; Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc: Mario Limonciello <mario.limonciello@dell.com>; open list:HID CORE LAYER <linux-input@vger.kernel.org>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; 斉藤 直樹 Naoki Saito <naoki.saito@alpsalpine.com>
Subject: RE: Alps touchpad generates IRQ storm after S3
Hi, Kai-Heng,
Sorry, I'm not in charge of Linux task now.
Hi, XiaoJian,
Please check the following mail.
If you have any question, please ask Kai-Heng.
Best Regards,
Masaki Ota
-----Original Message-----
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
Sent: Wednesday, August 28, 2019 3:22 PM
To: 太田 真喜 Masaki Ota <masaki.ota@alpsalpine.com>
Cc: Mario Limonciello <mario.limonciello@dell.com>; open list:HID CORE LAYER <linux-input@vger.kernel.org>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Alps touchpad generates IRQ storm after S3
Hi Masaki,
The Alps touchpad (044E:1220) on Dell Precision 7530 causes IRQ storm after system suspend (S3).
Commit "HID: i2c-hid: Don't reset device upon system resume” which solves the same issue for other vendors, cause the issue on Alps touchpad.
So I’d like to know the correct command Alps touchpad expects after system resume.
Also Cc Mario because this could relate to BIOS.
Kai-Heng
^ permalink raw reply
* Re: Alps touchpad generates IRQ storm after S3
From: Kai-Heng Feng @ 2019-08-28 6:58 UTC (permalink / raw)
To: Xiaojian Cao
Cc: Masaki Ota, Mario Limonciello, open list:HID CORE LAYER,
Linux Kernel Mailing List, Naoki Saito
In-Reply-To: <TYXPR01MB1470902D804A47EE72013006C8A30@TYXPR01MB1470.jpnprd01.prod.outlook.com>
Hi Xiaojian,
at 14:51, Xiaojian Cao <xiaojian.cao@cn.alps.com> wrote:
> Hi Ota-san,
>
> OK, we will look into it.
>
>
> Hi Kai-Heng,
>
> We will try to reproduce this issue first, could you please tell me the
> target Ubuntu version?
It’s distro-agnostic, any distro with mainline Linux can reproduce the issue.
Kai-Heng
>
> Best regards,
> Jason
>
> -----Original Message-----
> From: 太田 真喜 Masaki Ota <masaki.ota@alpsalpine.com>
> Sent: Wednesday, August 28, 2019 2:35 PM
> To: 曹 曉建 Xiaojian Cao <xiaojian.cao@cn.alps.com>; Kai-Heng Feng
> <kai.heng.feng@canonical.com>
> Cc: Mario Limonciello <mario.limonciello@dell.com>; open list:HID CORE
> LAYER <linux-input@vger.kernel.org>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>; 斉藤 直樹 Naoki Saito
> <naoki.saito@alpsalpine.com>
> Subject: RE: Alps touchpad generates IRQ storm after S3
>
> Hi, Kai-Heng,
>
> Sorry, I'm not in charge of Linux task now.
>
> Hi, XiaoJian,
>
> Please check the following mail.
> If you have any question, please ask Kai-Heng.
>
> Best Regards,
> Masaki Ota
> -----Original Message-----
> From: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Sent: Wednesday, August 28, 2019 3:22 PM
> To: 太田 真喜 Masaki Ota <masaki.ota@alpsalpine.com>
> Cc: Mario Limonciello <mario.limonciello@dell.com>; open list:HID CORE
> LAYER <linux-input@vger.kernel.org>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>
> Subject: Alps touchpad generates IRQ storm after S3
>
> Hi Masaki,
>
> The Alps touchpad (044E:1220) on Dell Precision 7530 causes IRQ storm
> after system suspend (S3).
> Commit "HID: i2c-hid: Don't reset device upon system resume” which solves
> the same issue for other vendors, cause the issue on Alps touchpad.
> So I’d like to know the correct command Alps touchpad expects after
> system resume.
>
> Also Cc Mario because this could relate to BIOS.
>
> Kai-Heng
^ permalink raw reply
* Re: [PATCH v2 1/2] input: keyboard: snvs_pwrkey: Send key events for i.MX6 S, DL and Q
From: Marco Felsch @ 2019-08-28 9:15 UTC (permalink / raw)
To: Robin van der Gracht
Cc: Robin Gong, Mark Rutland, devicetree @ vger . kernel . org,
Shawn Guo, Sascha Hauer, Dmitry Torokhov,
linux-kernel @ vger . kernel . org, Rob Herring, dl-linux-imx,
Pengutronix Kernel Team, linux-input @ vger . kernel . org,
Adam Ford, Fabio Estevam,
linux-arm-kernel @ lists . infradead . org
In-Reply-To: <20190827123216.32728-1-robin@protonic.nl>
Hi Robin,
thanks for the patch.
On 19-08-27 14:32, Robin van der Gracht wrote:
> The first generation i.MX6 processors does not send an interrupt when the
> power key is pressed. It sends a power down request interrupt if the key is
> released before a hard shutdown (5 second press). This should allow
> software to bring down the SoC safely.
>
> For this driver to work as a regular power key with the older SoCs, we need
> to send a keypress AND release when we get the power down request irq.
>
> Signed-off-by: Robin van der Gracht <robin@protonic.nl>
> ---
> .../devicetree/bindings/crypto/fsl-sec4.txt | 16 ++++--
> drivers/input/keyboard/Kconfig | 2 +-
> drivers/input/keyboard/snvs_pwrkey.c | 52 ++++++++++++++++---
Can we split this so the dt-bindings are a standalone patch? IMHO this
is the usual way because the maintainer can squash them on there needs.
Also it would be cool to document the changes. A common place for
changes is after the '---' or on the cover-letter.
> 3 files changed, 57 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
> index 2fe245ca816a..e4fbb9797082 100644
> --- a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
> +++ b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
> @@ -420,14 +420,22 @@ EXAMPLE
> =====================================================================
> System ON/OFF key driver
>
> - The snvs-pwrkey is designed to enable POWER key function which controlled
> - by SNVS ONOFF, the driver can report the status of POWER key and wakeup
> - system if pressed after system suspend.
> + The snvs-pwrkey is designed to enable POWER key function which is controlled
> + by SNVS ONOFF. It can wakeup the system if pressed after system suspend.
> +
> + There are two generations of SVNS pwrkey hardware. The first generation is
> + included in i.MX6 Solo, DualLite and Quad processors. The second generation
> + is included in i.MX6 SoloX and newer SoCs.
> +
> + Second generation SNVS can detect and report the status of POWER key, but the
> + first generation can only detect a key release and so emits an instantaneous
> + press and release event when the key is released.
>
> - compatible:
> Usage: required
> Value type: <string>
> - Definition: Mush include "fsl,sec-v4.0-pwrkey".
> + Definition: Must include "fsl,sec-v4.0-pwrkey" for i.MX6 SoloX and newer
> + or "fsl,imx6qdl-snvs-pwrkey" for older SoCs.
>
> - interrupts:
> Usage: required
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 7c4f19dab34f..937e58da5ce1 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -436,7 +436,7 @@ config KEYBOARD_SNVS_PWRKEY
> depends on OF
> help
> This is the snvs powerkey driver for the Freescale i.MX application
> - processors that are newer than i.MX6 SX.
> + processors.
>
> To compile this driver as a module, choose M here; the
> module will be called snvs_pwrkey.
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> index 5342d8d45f81..d71c44733103 100644
> --- a/drivers/input/keyboard/snvs_pwrkey.c
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -29,6 +29,11 @@
> #define DEBOUNCE_TIME 30
> #define REPEAT_INTERVAL 60
>
> +enum imx_snvs_hwtype {
> + IMX6SX_SNVS, /* i.MX6 SoloX and newer */
> + IMX6QDL_SNVS, /* i.MX6 Solo, DualLite and Quad */
> +};
> +
> struct pwrkey_drv_data {
> struct regmap *snvs;
> int irq;
> @@ -37,14 +42,41 @@ struct pwrkey_drv_data {
> int wakeup;
> struct timer_list check_timer;
> struct input_dev *input;
> + enum imx_snvs_hwtype hwtype;
> };
>
> +static const struct of_device_id imx_snvs_pwrkey_ids[] = {
> + {
> + .compatible = "fsl,sec-v4.0-pwrkey",
> + .data = (const void *)IMX6SX_SNVS,
> + },
> + {
> + .compatible = "fsl,imx6qdl-snvs-pwrkey",
> + .data = (const void *)IMX6QDL_SNVS,
> + },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids);
Can we keep this on the original place if you are using ...
> +
> static void imx_imx_snvs_check_for_events(struct timer_list *t)
> {
> struct pwrkey_drv_data *pdata = from_timer(pdata, t, check_timer);
> struct input_dev *input = pdata->input;
> u32 state;
>
> + if (pdata->hwtype == IMX6QDL_SNVS) {
> + /*
> + * The first generation i.MX6 SoCs only sends an interrupt on
> + * button release. To mimic power-key usage, we'll prepend a
> + * press event.
> + */
> + input_report_key(input, pdata->keycode, 1);
Missing input_sync() here?
> + input_report_key(input, pdata->keycode, 0);
> + input_sync(input);
> + pm_relax(input->dev.parent);
> + return;
> + }
> +
> regmap_read(pdata->snvs, SNVS_HPSR_REG, &state);
> state = state & SNVS_HPSR_BTN ? 1 : 0;
>
> @@ -67,13 +99,17 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id)
> {
> struct platform_device *pdev = dev_id;
> struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev);
> + unsigned long expire = jiffies;
> u32 lp_status;
>
> pm_wakeup_event(pdata->input->dev.parent, 0);
>
> regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
> - if (lp_status & SNVS_LPSR_SPO)
> - mod_timer(&pdata->check_timer, jiffies + msecs_to_jiffies(DEBOUNCE_TIME));
> + if (lp_status & SNVS_LPSR_SPO) {
> + if (pdata->hwtype == IMX6SX_SNVS)
> + expire += msecs_to_jiffies(DEBOUNCE_TIME);
> + mod_timer(&pdata->check_timer, expire);
Is this desired because the timer gets triggered earlier.
> + }
>
> /* clear SPO status */
> regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO);
> @@ -93,6 +129,7 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> struct pwrkey_drv_data *pdata = NULL;
> struct input_dev *input = NULL;
> struct device_node *np;
> + const struct of_device_id *match;
> int error;
>
> /* Get SNVS register Page */
> @@ -100,6 +137,10 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> if (!np)
> return -ENODEV;
>
> + match = of_match_node(imx_snvs_pwrkey_ids, np);
> + if (!match)
> + return -ENODEV;
... of_device_get_match_data() here. While reading the rm it seems that
the snvs block has a dedicated version register. IMHO this could be a
better way to apply the change also to existing devices with old
firmware.
Regards,
Marco
> +
> pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> if (!pdata)
> return -ENOMEM;
> @@ -115,6 +156,7 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n");
> }
>
> + pdata->hwtype = (enum imx_snvs_hwtype)match->data;
> pdata->wakeup = of_property_read_bool(np, "wakeup-source");
>
> pdata->irq = platform_get_irq(pdev, 0);
> @@ -175,12 +217,6 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> return 0;
> }
>
> -static const struct of_device_id imx_snvs_pwrkey_ids[] = {
> - { .compatible = "fsl,sec-v4.0-pwrkey" },
> - { /* sentinel */ }
> -};
> -MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids);
> -
> static struct platform_driver imx_snvs_pwrkey_driver = {
> .driver = {
> .name = "snvs_pwrkey",
> --
> 2.20.1
>
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* RE: Alps touchpad generates IRQ storm after S3
From: Mario.Limonciello @ 2019-08-28 13:25 UTC (permalink / raw)
To: kai.heng.feng, xiaojian.cao
Cc: masaki.ota, linux-input, linux-kernel, naoki.saito
In-Reply-To: <A118551C-A0D9-485F-91F7-44A5BE228B99@canonical.com>
KH,
Just make sure I understand details.
> Commit "HID: i2c-hid: Don't reset device upon system resume
If you revert this it's fixed on this system?
In that commit you had mentioned if this causes problems it might be worth
quirking just Raydium but commit afbb1169ed5b58cfca017e368b53e019cf285853
confirmed that it helped several other systems too.
If the conclusion from this investigation this is only fixable via touchpad FW update
it might be worth quirking this touchpad/touchpad FW/system combination.
> Also Cc Mario because this could relate to BIOS.
Also I assume this is on current stable BIOS/EC release, right?
Thanks,
> -----Original Message-----
> From: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Sent: Wednesday, August 28, 2019 1:58 AM
> To: Xiaojian Cao
> Cc: Masaki Ota; Limonciello, Mario; open list:HID CORE LAYER; Linux Kernel
> Mailing List; Naoki Saito
> Subject: Re: Alps touchpad generates IRQ storm after S3
>
>
> [EXTERNAL EMAIL]
>
> Hi Xiaojian,
>
> at 14:51, Xiaojian Cao <xiaojian.cao@cn.alps.com> wrote:
>
> > Hi Ota-san,
> >
> > OK, we will look into it.
> >
> >
> > Hi Kai-Heng,
> >
> > We will try to reproduce this issue first, could you please tell me the
> > target Ubuntu version?
>
> It’s distro-agnostic, any distro with mainline Linux can reproduce the issue.
>
> Kai-Heng
>
> >
> > Best regards,
> > Jason
> >
> > -----Original Message-----
> > From: 太田 真喜 Masaki Ota <masaki.ota@alpsalpine.com>
> > Sent: Wednesday, August 28, 2019 2:35 PM
> > To: 曹 曉建 Xiaojian Cao <xiaojian.cao@cn.alps.com>; Kai-Heng Feng
> > <kai.heng.feng@canonical.com>
> > Cc: Mario Limonciello <mario.limonciello@dell.com>; open list:HID CORE
> > LAYER <linux-input@vger.kernel.org>; Linux Kernel Mailing List
> > <linux-kernel@vger.kernel.org>; 斉藤 直樹 Naoki Saito
> > <naoki.saito@alpsalpine.com>
> > Subject: RE: Alps touchpad generates IRQ storm after S3
> >
> > Hi, Kai-Heng,
> >
> > Sorry, I'm not in charge of Linux task now.
> >
> > Hi, XiaoJian,
> >
> > Please check the following mail.
> > If you have any question, please ask Kai-Heng.
> >
> > Best Regards,
> > Masaki Ota
> > -----Original Message-----
> > From: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > Sent: Wednesday, August 28, 2019 3:22 PM
> > To: 太田 真喜 Masaki Ota <masaki.ota@alpsalpine.com>
> > Cc: Mario Limonciello <mario.limonciello@dell.com>; open list:HID CORE
> > LAYER <linux-input@vger.kernel.org>; Linux Kernel Mailing List
> > <linux-kernel@vger.kernel.org>
> > Subject: Alps touchpad generates IRQ storm after S3
> >
> > Hi Masaki,
> >
> > The Alps touchpad (044E:1220) on Dell Precision 7530 causes IRQ storm
> > after system suspend (S3).
> > Commit "HID: i2c-hid: Don't reset device upon system resume” which solves
> > the same issue for other vendors, cause the issue on Alps touchpad.
> > So I’d like to know the correct command Alps touchpad expects after
> > system resume.
> >
> > Also Cc Mario because this could relate to BIOS.
> >
> > Kai-Heng
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox