Hi Dmitry,
On Mon, Jul 25, 2016 at 3:48 PM, Marcos Souza
<marcos.souza.org@xxxxxxxxx> wrote:
> Hi Dmitry,
>
> On Mon, Jul 25, 2016 at 3:22 PM Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> wrote:
>>
>> [ resending to all - mutt is for some reason confused about recipent
>> list ]
>> On Mon, Jul 25, 2016 at 02:20:16PM -0300, Marcos Paulo de Souza wrote:
>> > On suspend/resume cycle, selftest is executed on to reset i8042
>> > controller. But
>> > when this is done in these devices, posterior calls to detect/init
>> > functions
>> > to elantech driver fails. Skipping selftest fixes this problem.
>> >
>> > An easier step to reproduce this problem is adding i8042.reset=1 as a
>> > kernel
>> > parameter. On problematic laptops, it'll make the system to start with
>> > the
>> > touchpad already stucked, since psmouse_probe forcibly calls the
>> > selftest function.
>> >
>> > This patch was inspired by John Hiesey's change[1].
>> >
>> > [1]: https://marc.info/?l=linux-input&m=144312209020616&w=2
>> >
>> > Fixes: "ETPS/2 Elantech Touchpad dies after resume from suspend"
>> > (https://bugzilla.kernel.org/show_bug.cgi?id=7739)
>>
>> *sigh* You just cant win, they'll manage to mess up the firmware one way
>> or another :( Witness:
>
>
> They're experts on messing things up :(
>
>>
>>
>> "
>> commit 1ca56e513a9fd356d5a9e0de45dbe0e189e00386
>> Author: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
>> Date: Tue Jul 20 20:25:34 2010 -0700
>>
>> Input: i8042 - reset keyboard controller wehen resuming from S2R
>>
>> Some laptops, such as Lenovo 3000 N100, require keyboard controller
>> reset
>> in order to have touchpad operable after suspend to RAM. Even if box
>> does
>> not need the reset it should be safe to do so, so instead of chasing
>> after misbehaving boxes and grow DMI tables, let's reset the
>> controller
>> unconditionally.
>>
>> Reported-and-tested-by: Jerome Lacoste <jerome.lacoste@xxxxxxxxx>
>> Signed-off-by: Dmitry Torokhov <dtor@xxxxxxx>
>> "
>>
>> so apparently it is not safe to reset the controller on resume. Oh joy.
>>
>> The issue I have here is selftest is the same as reset, and we already
>> have i8042_reset flag, so 2 flags controlling the same functionality is
>> not great. Could we extend i8042 to an enum, like:
>>
>> I8042_RESET_ALWAYS
>> I8042_RESET_NEVER
>> I8042_RESET_ON_RESUME (default)
>>
>> and have a custom module parameter for it?
>>
>> i8042.reset and i8042_reset={1|Y|y} would result in I8042_RESET_ALWAYS
>> i8042.reset={0|N|n} would result in I8042_RESET_NEVER
>> Without i8042.reset we just reset it when resuming from S3.
>>
>> What do you think?
>
>
> Seems to be a very good approach. I'll send a new version.
You asked me to adapt my previous patch, making it accept:
i8042.reset
i8042.reset={1,y,Y}
i8042.reset={0,n,N}
But, when using a charp as module parameter, it doesn't allocate
memory to i8042_reset when we don't use the equal sign. So, in this
case I can't tell if the user is using i8042.reset, or don't any
parameter (in this case it means to use I8042_RESET_ON_RESUME)
So, in this case, should we just change i8042.reset to accept 1,y,n?
This should solve the problem too (although I think people who already
use such parameter will complain...)
What do you think?
I attached my current diff here, by using a new kernel parameter
reset_mode. Besides the new parameter,It works great if I remove the
comment about the ASUS DMI info.
Thanks,
>
>>
>>
>> Thanks.
>>
>> --
>> Dmitry
--
Att,
Marcos Paulo de Souza
Github: https://github.com/marcosps/
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 82b42c9..ac0d5cd 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1431,6 +1431,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
controllers
i8042.notimeout [HW] Ignore timeout condition signalled by controller
i8042.reset [HW] Reset the controller during init and cleanup
+ i8042.reset_mode
+ [HW] Reset the controller, always, never or just on
+ resume
+ Format: { 1 | Y | y | 0 | N | n }
+ 1, Y, y: Always reset controller (same as the i8042.reset)
+ 0, N, n: Never reset controller
+ Default: Just reset on S3 resume
i8042.unlock [HW] Unlock (ignore) the keylock
i8042.kbdreset [HW] Reset device connected to KBD port
diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index 68f5f4a..40a78d1 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -510,6 +510,60 @@ static const struct dmi_system_id __initconst i8042_dmi_nomux_table[] = {
{ }
};
+/*
+ * On some hardware just running the self test causes problems.
+ */
+static const struct dmi_system_id __initconst i8042_dmi_noselftest_table[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "K401LB"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "K501LX"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "K501LD"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "X302LA"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "X450LCP"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "X450LD"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "X455LAB"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "X455LDB"),
+ },
+ },
+ { }
+};
static const struct dmi_system_id __initconst i8042_dmi_reset_table[] = {
{
/* MSI Wind U-100 */
@@ -1077,7 +1131,11 @@ static int __init i8042_platform_init(void)
#ifdef CONFIG_X86
if (dmi_check_system(i8042_dmi_reset_table))
- i8042_reset = true;
+ i8042_reset_mode = I8042_RESET_ALWAYS;
+
+ // just disable controller reset
+ //if (dmi_check_system(i8042_dmi_noselftest_table))
+ // i8042_reset_mode = I8042_RESET_NEVER;
if (dmi_check_system(i8042_dmi_noloop_table))
i8042_noloop = true;
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 4541957..cfc1590 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -52,6 +52,17 @@ static bool i8042_reset;
module_param_named(reset, i8042_reset, bool, 0);
MODULE_PARM_DESC(reset, "Reset controller during init and cleanup.");
+enum i8042_controller_reset_mode {
+ I8042_RESET_NEVER,
+ I8042_RESET_ALWAYS,
+ I8042_RESET_ON_RESUME
+};
+
+static unsigned int i8042_reset_mode = I8042_RESET_ON_RESUME;
+static char *i8042_reset_param;
+module_param_named(reset_mode, i8042_reset_param, charp, 0);
+MODULE_PARM_DESC(reset_mode, "Reset controller mode, being always, never or just on resume");
+
static bool i8042_direct;
module_param_named(direct, i8042_direct, bool, 0);
MODULE_PARM_DESC(direct, "Put keyboard port into non-translated mode.");
@@ -890,6 +901,9 @@ static int i8042_controller_selftest(void)
unsigned char param;
int i = 0;
+ if (i8042_reset_mode == I8042_RESET_NEVER)
+ return 0;
+
/*
* We try this 5 times; on some really fragile systems this does not
* take the first time...
@@ -1495,7 +1509,20 @@ static int __init i8042_probe(struct platform_device *dev)
i8042_platform_device = dev;
- if (i8042_reset) {
+ if (i8042_reset_param != NULL) {
+ if (!strncmp(i8042_reset_param, "1", 1)
+ || !strncasecmp(i8042_reset_param, "y", 1)) {
+ i8042_reset_mode = I8042_RESET_ALWAYS;
+ } else if (!strncmp(i8042_reset_param, "0", 1)
+ || !strncasecmp(i8042_reset_param, "n", 1)) {
+ i8042_reset_mode = I8042_RESET_NEVER;
+ }
+ }
+
+ if (i8042_reset)
+ i8042_reset_mode = I8042_RESET_ALWAYS;
+
+ if (i8042_reset_mode == I8042_RESET_ALWAYS) {
error = i8042_controller_selftest();
if (error)
return error;
